fix: upstream security and robustness fixes (MACThenDecrypt alignment, CLI buffer, packet pool, raw data paths, ESP32 time after reset, Bosch sensor address)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Jakub
2026-10-12 19:53:53 +02:00
co-authored by Claude Opus 5.5
parent ec42a74ab6
commit 18f5627b2a
11 changed files with 130 additions and 35 deletions
+23 -11
View File
@@ -2065,16 +2065,20 @@ void MyMesh::handleCmdFrame(size_t len) {
#endif #endif
} else if (cmd_frame[0] == CMD_SEND_RAW_DATA && len >= 6) { } else if (cmd_frame[0] == CMD_SEND_RAW_DATA && len >= 6) {
int i = 1; int i = 1;
int8_t path_len = cmd_frame[i++]; uint8_t path_len = cmd_frame[i++]; // encoded: hash size in the top bits (upstream 0cce9197 / 67d6d42a)
if (path_len >= 0 && i + path_len + 4 <= len) { // minimum 4 byte payload if (mesh::Packet::isValidPathLen(path_len)) {
uint8_t *path = &cmd_frame[i]; uint8_t path[MAX_PATH_SIZE];
i += path_len; i += mesh::Packet::writePath(path, &cmd_frame[i], path_len);
auto pkt = createRawData(&cmd_frame[i], len - i); if (i + 4 > len) { // min payload 4 bytes
if (pkt) { writeErrFrame(ERR_CODE_ILLEGAL_ARG);
sendDirect(pkt, path, path_len);
writeOKFrame();
} else { } else {
writeErrFrame(ERR_CODE_TABLE_FULL); auto pkt = createRawData(&cmd_frame[i], len - i);
if (pkt) {
sendDirect(pkt, path, path_len);
writeOKFrame();
} else {
writeErrFrame(ERR_CODE_TABLE_FULL);
}
} }
} else { } else {
writeErrFrame(ERR_CODE_UNSUPPORTED_CMD); // flood, not supported (yet) writeErrFrame(ERR_CODE_UNSUPPORTED_CMD); // flood, not supported (yet)
@@ -2634,6 +2638,13 @@ void MyMesh::enterCLIRescue() {
void MyMesh::checkCLIRescueCmd() { void MyMesh::checkCLIRescueCmd() {
int len = strlen(cli_command); int len = strlen(cli_command);
// `cli_command` must stay NUL-terminated within its bounds. If it ever isn't,
// strlen() above can return >= sizeof(cli_command) and the loop below would
// then index past the buffer, so clamp defensively.
if (len >= (int)sizeof(cli_command)) {
cli_command[0] = 0;
len = 0;
}
while (Serial.available() && len < sizeof(cli_command)-1) { while (Serial.available() && len < sizeof(cli_command)-1) {
char c = Serial.read(); char c = Serial.read();
if (c != '\n') { if (c != '\n') {
@@ -2642,8 +2653,9 @@ void MyMesh::checkCLIRescueCmd() {
} }
Serial.print(c); // echo Serial.print(c); // echo
} }
if (len == sizeof(cli_command)-1) { // command buffer full if (len == sizeof(cli_command)-1) { // buffer full: treat as a completed line
cli_command[sizeof(cli_command)-1] = '\r'; cli_command[sizeof(cli_command)-2] = '\r'; // place end-of-line marker inside the buffer
cli_command[sizeof(cli_command)-1] = 0; // keep the buffer NUL-terminated
} }
if (len > 0 && cli_command[len - 1] == '\r') { // received complete line if (len > 0 && cli_command[len - 1] == '\r') { // received complete line
+10 -2
View File
@@ -204,6 +204,13 @@ void setup() {
void loop() { void loop() {
// Handle Serial CLI // Handle Serial CLI
int len = strlen(command); int len = strlen(command);
// `command` must stay NUL-terminated within its bounds. If it ever isn't,
// strlen() above can return >= sizeof(command) and the loop below would then
// index past the buffer, so clamp defensively.
if (len >= (int)sizeof(command)) {
command[0] = 0;
len = 0;
}
while (Serial.available() && len < sizeof(command)-1) { while (Serial.available() && len < sizeof(command)-1) {
char c = Serial.read(); char c = Serial.read();
if (c != '\n') { if (c != '\n') {
@@ -213,8 +220,9 @@ void loop() {
} }
if (c == '\r') break; if (c == '\r') break;
} }
if (len == sizeof(command)-1) { // command buffer full if (len == sizeof(command)-1) { // buffer full: treat as a completed line
command[sizeof(command)-1] = '\r'; command[sizeof(command)-2] = '\r'; // place end-of-line marker inside the buffer
command[sizeof(command)-1] = 0; // keep the buffer NUL-terminated
} }
if (len > 0 && command[len - 1] == '\r') { // received complete line if (len > 0 && command[len - 1] == '\r') { // received complete line
+10 -2
View File
@@ -142,6 +142,13 @@ void setup() {
void loop() { void loop() {
int len = strlen(command); int len = strlen(command);
// `command` must stay NUL-terminated within its bounds. If it ever isn't,
// strlen() above can return >= sizeof(command) and the loop below would then
// index past the buffer, so clamp defensively.
if (len >= (int)sizeof(command)) {
command[0] = 0;
len = 0;
}
while (Serial.available() && len < sizeof(command)-1) { while (Serial.available() && len < sizeof(command)-1) {
char c = Serial.read(); char c = Serial.read();
if (c != '\n') { if (c != '\n') {
@@ -150,8 +157,9 @@ void loop() {
} }
Serial.print(c); Serial.print(c);
} }
if (len == sizeof(command)-1) { // command buffer full if (len == sizeof(command)-1) { // buffer full: treat as a completed line
command[sizeof(command)-1] = '\r'; command[sizeof(command)-2] = '\r'; // place end-of-line marker inside the buffer
command[sizeof(command)-1] = 0; // keep the buffer NUL-terminated
} }
if (len > 0 && command[len - 1] == '\r') { // received complete line if (len > 0 && command[len - 1] == '\r') { // received complete line
+10 -2
View File
@@ -529,6 +529,13 @@ public:
BaseChatMesh::loop(); BaseChatMesh::loop();
int len = strlen(command); int len = strlen(command);
// `command` must stay NUL-terminated within its bounds. If it ever isn't,
// strlen() above can return >= sizeof(command) and the loop below would then
// index past the buffer, so clamp defensively.
if (len >= (int)sizeof(command)) {
command[0] = 0;
len = 0;
}
while (Serial.available() && len < sizeof(command)-1) { while (Serial.available() && len < sizeof(command)-1) {
char c = Serial.read(); char c = Serial.read();
if (c != '\n') { if (c != '\n') {
@@ -537,8 +544,9 @@ public:
} }
Serial.print(c); Serial.print(c);
} }
if (len == sizeof(command)-1) { // command buffer full if (len == sizeof(command)-1) { // buffer full: treat as a completed line
command[sizeof(command)-1] = '\r'; command[sizeof(command)-2] = '\r'; // place end-of-line marker inside the buffer
command[sizeof(command)-1] = 0; // keep the buffer NUL-terminated
} }
if (len > 0 && command[len - 1] == '\r') { // received complete line if (len > 0 && command[len - 1] == '\r') { // received complete line
+10 -2
View File
@@ -122,6 +122,13 @@ void setup() {
void loop() { void loop() {
int len = strlen(command); int len = strlen(command);
// `command` must stay NUL-terminated within its bounds. If it ever isn't,
// strlen() above can return >= sizeof(command) and the loop below would then
// index past the buffer, so clamp defensively.
if (len >= (int)sizeof(command)) {
command[0] = 0;
len = 0;
}
while (Serial.available() && len < sizeof(command)-1) { while (Serial.available() && len < sizeof(command)-1) {
char c = Serial.read(); char c = Serial.read();
if (c != '\n') { if (c != '\n') {
@@ -130,8 +137,9 @@ void loop() {
} }
Serial.print(c); Serial.print(c);
} }
if (len == sizeof(command)-1) { // command buffer full if (len == sizeof(command)-1) { // buffer full: treat as a completed line
command[sizeof(command)-1] = '\r'; command[sizeof(command)-2] = '\r'; // place end-of-line marker inside the buffer
command[sizeof(command)-1] = 0; // keep the buffer NUL-terminated
} }
if (len > 0 && command[len - 1] == '\r') { // received complete line if (len > 0 && command[len - 1] == '\r') { // received complete line
+8 -3
View File
@@ -147,13 +147,18 @@ int Utils::encryptThenMAC(const uint8_t* shared_secret, uint8_t* dest, const uin
int Utils::MACThenDecrypt(const uint8_t* shared_secret, uint8_t* dest, const uint8_t* src, int src_len) { int Utils::MACThenDecrypt(const uint8_t* shared_secret, uint8_t* dest, const uint8_t* src, int src_len) {
if (src_len <= CIPHER_MAC_SIZE) return 0; // invalid src bytes if (src_len <= CIPHER_MAC_SIZE) return 0; // invalid src bytes
// decrypt() works in whole blocks: an unaligned length would write past
// the caller's buffer (upstream 9c9a2ba2 / b74ba545).
int enc_len = src_len - CIPHER_MAC_SIZE;
if (enc_len % CIPHER_BLOCK_SIZE != 0) return 0; // reject non-block-aligned ciphertext
uint8_t hmac[CIPHER_MAC_SIZE]; uint8_t hmac[CIPHER_MAC_SIZE];
#ifdef USE_CC310_HW_CRYPTO #ifdef USE_CC310_HW_CRYPTO
{ {
static CRYS_HMACUserContext_t hmac_ctx; static CRYS_HMACUserContext_t hmac_ctx;
static CRYS_HASH_Result_t hmac_result; static CRYS_HASH_Result_t hmac_result;
CRYS_HMAC_Init(&hmac_ctx, CRYS_HASH_SHA256_mode, (uint8_t*)shared_secret, PUB_KEY_SIZE); CRYS_HMAC_Init(&hmac_ctx, CRYS_HASH_SHA256_mode, (uint8_t*)shared_secret, PUB_KEY_SIZE);
CRYS_HMAC_Update(&hmac_ctx, (uint8_t*)(src + CIPHER_MAC_SIZE), src_len - CIPHER_MAC_SIZE); CRYS_HMAC_Update(&hmac_ctx, (uint8_t*)(src + CIPHER_MAC_SIZE), enc_len);
CRYS_HMAC_Finish(&hmac_ctx, hmac_result); CRYS_HMAC_Finish(&hmac_ctx, hmac_result);
memcpy(hmac, hmac_result, CIPHER_MAC_SIZE); memcpy(hmac, hmac_result, CIPHER_MAC_SIZE);
} }
@@ -161,12 +166,12 @@ int Utils::MACThenDecrypt(const uint8_t* shared_secret, uint8_t* dest, const uin
{ {
SHA256 sha; SHA256 sha;
sha.resetHMAC(shared_secret, PUB_KEY_SIZE); sha.resetHMAC(shared_secret, PUB_KEY_SIZE);
sha.update(src + CIPHER_MAC_SIZE, src_len - CIPHER_MAC_SIZE); sha.update(src + CIPHER_MAC_SIZE, enc_len);
sha.finalizeHMAC(shared_secret, PUB_KEY_SIZE, hmac, CIPHER_MAC_SIZE); sha.finalizeHMAC(shared_secret, PUB_KEY_SIZE, hmac, CIPHER_MAC_SIZE);
} }
#endif #endif
if (memcmp(hmac, src, CIPHER_MAC_SIZE) == 0) { if (memcmp(hmac, src, CIPHER_MAC_SIZE) == 0) {
return decrypt(shared_secret, dest, src + CIPHER_MAC_SIZE, src_len - CIPHER_MAC_SIZE); return decrypt(shared_secret, dest, src + CIPHER_MAC_SIZE, enc_len);
} }
return 0; // invalid HMAC return 0; // invalid HMAC
} }
+1 -1
View File
@@ -50,7 +50,7 @@ public:
/** /**
* \brief checks the MAC (in leading bytes of 'src'), then if valid, decrypts remaining bytes in src. * \brief checks the MAC (in leading bytes of 'src'), then if valid, decrypts remaining bytes in src.
* \returns zero if MAC is invalid, otherwise the length of decrypted bytes in 'dest' * \returns zero if MAC is invalid or ciphertext is not block-aligned, otherwise the length of decrypted bytes in 'dest'
*/ */
static int MACThenDecrypt(const uint8_t* shared_secret, uint8_t* dest, const uint8_t* src, int src_len); static int MACThenDecrypt(const uint8_t* shared_secret, uint8_t* dest, const uint8_t* src, int src_len);
+26 -5
View File
@@ -197,19 +197,30 @@ public:
} }
}; };
static RTC_NOINIT_ATTR uint32_t _rtc_backup_time;
static RTC_NOINIT_ATTR uint32_t _rtc_backup_magic;
#define RTC_BACKUP_MAGIC 0xAA55CC33
#define RTC_TIME_MIN 1772323200 // 1 Mar 2026
class ESP32RTCClock : public mesh::RTCClock { class ESP32RTCClock : public mesh::RTCClock {
public: public:
ESP32RTCClock() { } ESP32RTCClock() { }
void begin() { void begin() {
esp_reset_reason_t reason = esp_reset_reason(); esp_reset_reason_t reason = esp_reset_reason();
if (reason == ESP_RST_POWERON) { if (reason == ESP_RST_DEEPSLEEP) {
// start with some date/time in the recent past return; // ESP-IDF preserves system time across deep sleep
struct timeval tv; }
tv.tv_sec = 1715770351; // 15 May 2024, 8:50pm // All other resets (power-on, crash, WDT, brownout) lose system time.
// Restore from RTC backup if valid, otherwise use hardcoded seed.
struct timeval tv;
if (_rtc_backup_magic == RTC_BACKUP_MAGIC && _rtc_backup_time > RTC_TIME_MIN) {
tv.tv_sec = _rtc_backup_time;
} else {
tv.tv_sec = 1772323200; // 1 Mar 2026
}
tv.tv_usec = 0; tv.tv_usec = 0;
settimeofday(&tv, NULL); settimeofday(&tv, NULL);
} }
}
uint32_t getCurrentTime() override { uint32_t getCurrentTime() override {
time_t _now; time_t _now;
time(&_now); time(&_now);
@@ -220,6 +231,16 @@ public:
tv.tv_sec = time; tv.tv_sec = time;
tv.tv_usec = 0; tv.tv_usec = 0;
settimeofday(&tv, NULL); settimeofday(&tv, NULL);
_rtc_backup_time = time;
_rtc_backup_magic = RTC_BACKUP_MAGIC;
}
void tick() override {
time_t now;
time(&now);
if (now > RTC_TIME_MIN && (uint32_t)now != _rtc_backup_time) {
_rtc_backup_time = (uint32_t)now;
_rtc_backup_magic = RTC_BACKUP_MAGIC;
}
} }
}; };
+7 -2
View File
@@ -44,7 +44,7 @@ mesh::Packet* PacketQueue::get(uint32_t now) {
} }
mesh::Packet* PacketQueue::removeByIdx(int i) { mesh::Packet* PacketQueue::removeByIdx(int i) {
if (i >= _num) return NULL; // invalid index if (i < 0 || i >= _num) return NULL; // invalid index
mesh::Packet* item = _table[i]; mesh::Packet* item = _table[i];
_num--; _num--;
@@ -80,10 +80,14 @@ mesh::Packet* StaticPoolPacketManager::allocNew() {
} }
void StaticPoolPacketManager::free(mesh::Packet* packet) { void StaticPoolPacketManager::free(mesh::Packet* packet) {
unused.add(packet, 0, 0); if (packet == NULL) return;
if (!unused.add(packet, 0, 0)) {
MESH_DEBUG_PRINTLN("free: unused queue full, possible double-free detected");
}
} }
void StaticPoolPacketManager::queueOutbound(mesh::Packet* packet, uint8_t priority, uint32_t scheduled_for) { void StaticPoolPacketManager::queueOutbound(mesh::Packet* packet, uint8_t priority, uint32_t scheduled_for) {
if (packet == NULL) return;
if (!send_queue.add(packet, priority, scheduled_for)) { if (!send_queue.add(packet, priority, scheduled_for)) {
MESH_DEBUG_PRINTLN("queueOutbound: send queue full, dropping packet"); MESH_DEBUG_PRINTLN("queueOutbound: send queue full, dropping packet");
free(packet); free(packet);
@@ -115,6 +119,7 @@ mesh::Packet* StaticPoolPacketManager::removeOutboundByIdx(int i) {
} }
void StaticPoolPacketManager::queueInbound(mesh::Packet* packet, uint32_t scheduled_for) { void StaticPoolPacketManager::queueInbound(mesh::Packet* packet, uint32_t scheduled_for) {
if (packet == NULL) return;
if (!rx_queue.add(packet, 0, scheduled_for)) { if (!rx_queue.add(packet, 0, scheduled_for)) {
MESH_DEBUG_PRINTLN("queueInbound: rx queue full, dropping packet"); MESH_DEBUG_PRINTLN("queueInbound: rx queue full, dropping packet");
free(packet); free(packet);
+4 -1
View File
@@ -14,7 +14,10 @@ public:
bool add(mesh::Packet* packet, uint8_t priority, uint32_t scheduled_for); bool add(mesh::Packet* packet, uint8_t priority, uint32_t scheduled_for);
int count() const { return _num; } int count() const { return _num; }
int countBefore(uint32_t now) const; int countBefore(uint32_t now) const;
mesh::Packet* itemAt(int i) const { return _table[i]; } mesh::Packet* itemAt(int i) const {
if (i < 0 || i >= _num) return NULL;
return _table[i];
}
mesh::Packet* removeByIdx(int i); mesh::Packet* removeByIdx(int i);
}; };
@@ -539,6 +539,8 @@ static void query_bme680_bsec(uint8_t ch, uint8_t, CayenneLPP& lpp) {
// are compiled in. The sentinel at the end keeps the array // are compiled in. The sentinel at the end keeps the array
// non-empty regardless of which sensors are enabled. // non-empty regardless of which sensors are enabled.
// //
// Bosch BMP/BME SDO selects 0x76 or 0x77; probe both.
//
// Ordering here determines channel assignment at runtime: // Ordering here determines channel assignment at runtime:
// the first detected+initialized sensor gets channel 2, the // the first detected+initialized sensor gets channel 2, the
// next gets channel 3, and so on. // next gets channel 3, and so on.
@@ -551,21 +553,27 @@ struct SensorDef {
void (*query)(uint8_t channel, uint8_t sub_channel, CayenneLPP& telemetry); void (*query)(uint8_t channel, uint8_t sub_channel, CayenneLPP& telemetry);
}; };
#define TELEM_BOSCH_ALT_ADDR(addr) ((uint8_t)((addr) == 0x76 ? 0x77 : 0x76))
static const SensorDef SENSOR_TABLE[] = { static const SensorDef SENSOR_TABLE[] = {
#if ENV_INCLUDE_AHTX0 #if ENV_INCLUDE_AHTX0
{ TELEM_AHTX_ADDRESS, "AHT10/AHT20", init_ahtx0, query_ahtx0 }, { TELEM_AHTX_ADDRESS, "AHT10/AHT20", init_ahtx0, query_ahtx0 },
#endif #endif
#ifdef ENV_INCLUDE_BME680 #ifdef ENV_INCLUDE_BME680
{ TELEM_BME680_ADDRESS, "BME680", init_bme680, query_bme680 }, { TELEM_BME680_ADDRESS, "BME680", init_bme680, query_bme680 },
{ TELEM_BOSCH_ALT_ADDR(TELEM_BME680_ADDRESS), "BME680", init_bme680, query_bme680 },
#endif #endif
#if ENV_INCLUDE_BME680_BSEC #if ENV_INCLUDE_BME680_BSEC
{ TELEM_BME680_ADDRESS, "BME680+BSEC", init_bme680_bsec, query_bme680_bsec }, { TELEM_BME680_ADDRESS, "BME680+BSEC", init_bme680_bsec, query_bme680_bsec },
{ TELEM_BOSCH_ALT_ADDR(TELEM_BME680_ADDRESS), "BME680+BSEC", init_bme680_bsec, query_bme680_bsec },
#endif #endif
#if ENV_INCLUDE_BME280 #if ENV_INCLUDE_BME280
{ TELEM_BME280_ADDRESS, "BME280", init_bme280, query_bme280 }, { TELEM_BME280_ADDRESS, "BME280", init_bme280, query_bme280 },
{ TELEM_BOSCH_ALT_ADDR(TELEM_BME280_ADDRESS), "BME280", init_bme280, query_bme280 },
#endif #endif
#if ENV_INCLUDE_BMP280 #if ENV_INCLUDE_BMP280
{ TELEM_BMP280_ADDRESS, "BMP280", init_bmp280, query_bmp280 }, { TELEM_BMP280_ADDRESS, "BMP280", init_bmp280, query_bmp280 },
{ TELEM_BOSCH_ALT_ADDR(TELEM_BMP280_ADDRESS), "BMP280", init_bmp280, query_bmp280 },
#endif #endif
#if ENV_INCLUDE_SHTC3 #if ENV_INCLUDE_SHTC3
{ 0x70, "SHTC3", init_shtc3, query_shtc3 }, { 0x70, "SHTC3", init_shtc3, query_shtc3 },
@@ -603,6 +611,8 @@ static const SensorDef SENSOR_TABLE[] = {
{ 0, nullptr, nullptr, nullptr } // sentinel — keeps the array non-empty { 0, nullptr, nullptr, nullptr } // sentinel — keeps the array non-empty
}; };
#undef TELEM_BOSCH_ALT_ADDR
static const size_t SENSOR_TABLE_SIZE = (sizeof(SENSOR_TABLE) / sizeof(SENSOR_TABLE[0])) - 1; static const size_t SENSOR_TABLE_SIZE = (sizeof(SENSOR_TABLE) / sizeof(SENSOR_TABLE[0])) - 1;
// ============================================================ // ============================================================
@@ -640,6 +650,12 @@ bool EnvironmentSensorManager::begin() {
_active_sensor_count = 0; _active_sensor_count = 0;
for (size_t i = 0; i < SENSOR_TABLE_SIZE && _active_sensor_count < MAX_ACTIVE_SENSORS; i++) { for (size_t i = 0; i < SENSOR_TABLE_SIZE && _active_sensor_count < MAX_ACTIVE_SENSORS; i++) {
const SensorDef& def = SENSOR_TABLE[i]; const SensorDef& def = SENSOR_TABLE[i];
// One static driver instance per type: an alternate address is a fallback, not a second device.
bool already_active = false;
for (int j = 0; j < _active_sensor_count; j++) {
if (_active_sensors[j].query == def.query) { already_active = true; break; }
}
if (already_active) continue;
if (!detected[def.address]) { if (!detected[def.address]) {
MESH_DEBUG_PRINTLN("%s not detected at I2C address %02X", def.name, def.address); MESH_DEBUG_PRINTLN("%s not detected at I2C address %02X", def.name, def.address);
continue; continue;
@@ -650,6 +666,7 @@ bool EnvironmentSensorManager::begin() {
continue; continue;
} }
MESH_DEBUG_PRINTLN("Found %s at address: %02X", def.name, def.address); MESH_DEBUG_PRINTLN("Found %s at address: %02X", def.name, def.address);
detected[def.address] = false; // consumed; later entries must not re-claim this device
for (uint8_t sub = 0; sub < n && _active_sensor_count < MAX_ACTIVE_SENSORS; sub++) { for (uint8_t sub = 0; sub < n && _active_sensor_count < MAX_ACTIVE_SENSORS; sub++) {
_active_sensors[_active_sensor_count++] = { def.query, sub }; _active_sensors[_active_sensor_count++] = { def.query, sub };
} }