Protocol parser fixes and ASAN

examples/simple_repeater/MyMesh.cpp

@@ -144,11 +144,12 @@ uint8_t MyMesh::handleLoginReq(const mesh::Identity& sender, const uint8_t* secr
   return 13;  // reply length
 }
 
-uint8_t MyMesh::handleAnonRegionsReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data) {
+uint8_t MyMesh::handleAnonRegionsReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data,
+                                     size_t data_len) {
   if (anon_limiter.allow(rtc_clock.getCurrentTime())) {
     // request data has: {reply-path-len}{reply-path}
+    if (!mesh::Packet::hasCompletePath(data, data_len)) return 0;
     reply_path_len = *data++;
-    if (!mesh::Packet::isValidPathLen(reply_path_len)) return 0;  // reject - bad encoding
 
     mesh::Packet::writePath(reply_path, data, reply_path_len);
     // data += (uint8_t)reply_path_len * reply_path_hash_size;
@@ -162,11 +163,12 @@ uint8_t MyMesh::handleAnonRegionsReq(const mesh::Identity& sender, uint32_t send
   return 0;
 }
 
-uint8_t MyMesh::handleAnonOwnerReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data) {
+uint8_t MyMesh::handleAnonOwnerReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data,
+                                   size_t data_len) {
   if (anon_limiter.allow(rtc_clock.getCurrentTime())) {
     // request data has: {reply-path-len}{reply-path}
+    if (!mesh::Packet::hasCompletePath(data, data_len)) return 0;
     reply_path_len = *data++;
-    if (!mesh::Packet::isValidPathLen(reply_path_len)) return 0;  // reject - bad encoding
 
     mesh::Packet::writePath(reply_path, data, reply_path_len);
     // data += (uint8_t)reply_path_len * reply_path_hash_size;
@@ -181,11 +183,12 @@ uint8_t MyMesh::handleAnonOwnerReq(const mesh::Identity& sender, uint32_t sender
   return 0;
 }
 
-uint8_t MyMesh::handleAnonClockReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data) {
+uint8_t MyMesh::handleAnonClockReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data,
+                                   size_t data_len) {
   if (anon_limiter.allow(rtc_clock.getCurrentTime())) {
     // request data has: {reply-path-len}{reply-path}
+    if (!mesh::Packet::hasCompletePath(data, data_len)) return 0;
     reply_path_len = *data++;
-    if (!mesh::Packet::isValidPathLen(reply_path_len)) return 0;  // reject - bad encoding
 
     mesh::Packet::writePath(reply_path, data, reply_path_len);
     // data += (uint8_t)reply_path_len * reply_path_hash_size;
@@ -572,6 +575,8 @@ void MyMesh::onAnonDataRecv(mesh::Packet *packet, const uint8_t *secret, const m
                             uint8_t *data, size_t len) {
   if (packet->getPayloadType() == PAYLOAD_TYPE_ANON_REQ) { // received an initial request by a possible admin
                                                            // client (unknown at this stage)
+    if (len < 5 || len >= MAX_PACKET_PAYLOAD) return;
+
     uint32_t timestamp;
     memcpy(&timestamp, data, 4);
 
@@ -582,11 +587,11 @@ void MyMesh::onAnonDataRecv(mesh::Packet *packet, const uint8_t *secret, const m
     if (data[4] == 0 || data[4] >= ' ') {   // is password, ie. a login request
       reply_len = handleLoginReq(sender, secret, timestamp, &data[4], packet->isRouteFlood());
     } else if (data[4] == ANON_REQ_TYPE_REGIONS && packet->isRouteDirect()) {
-      reply_len = handleAnonRegionsReq(sender, timestamp, &data[5]);
+      reply_len = handleAnonRegionsReq(sender, timestamp, &data[5], len - 5);
     } else if (data[4] == ANON_REQ_TYPE_OWNER && packet->isRouteDirect()) {
-      reply_len = handleAnonOwnerReq(sender, timestamp, &data[5]);
+      reply_len = handleAnonOwnerReq(sender, timestamp, &data[5], len - 5);
     } else if (data[4] == ANON_REQ_TYPE_BASIC && packet->isRouteDirect()) {
-      reply_len = handleAnonClockReq(sender, timestamp, &data[5]);
+      reply_len = handleAnonClockReq(sender, timestamp, &data[5], len - 5);
     } else {
       reply_len = 0;  // unknown/invalid request type
     }

examples/simple_repeater/MyMesh.h

@@ -118,9 +118,9 @@ class MyMesh : public mesh::Mesh, public CommonCLICallbacks {
 
   void putNeighbour(const mesh::Identity& id, uint32_t timestamp, float snr);
   uint8_t handleLoginReq(const mesh::Identity& sender, const uint8_t* secret, uint32_t sender_timestamp, const uint8_t* data, bool is_flood);
-  uint8_t handleAnonRegionsReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data);
-  uint8_t handleAnonOwnerReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data);
-  uint8_t handleAnonClockReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data);
+  uint8_t handleAnonRegionsReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data, size_t data_len);
+  uint8_t handleAnonOwnerReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data, size_t data_len);
+  uint8_t handleAnonClockReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data, size_t data_len);
   int handleRequest(ClientInfo* sender, uint32_t sender_timestamp, uint8_t* payload, size_t payload_len);
   mesh::Packet* createSelfAdvert();
 

platformio.ini

@@ -172,11 +172,21 @@ build_src_filter =
   -<*>
   +<../src/Utils.cpp>
   +<../src/Packet.cpp>
+  +<../src/helpers/AdvertDataHelpers.cpp>
   +<../src/helpers/ConfigSerializer.cpp>
   +<../src/helpers/DynamicConfigSerializer.cpp>
 lib_deps =
   google/googletest @ 1.17.0
 
+[env:native_asan]
+extends = env:native
+build_unflags = -Os
+build_flags = ${env:native.build_flags}
+  -O1
+  -g
+  -fsanitize=address,undefined
+  -fno-omit-frame-pointer
+
 [env:native_kiss_modem]
 platform = native
 test_framework = googletest

src/Dispatcher.cpp

@@ -147,45 +147,11 @@ void Dispatcher::loop() {
 }
 
 bool Dispatcher::tryParsePacket(Packet* pkt, const uint8_t* raw, int len) {
-  int i = 0;
-
-  pkt->header = raw[i++];
-  if (pkt->getPayloadVer() > PAYLOAD_VER_1) {
-    MESH_DEBUG_PRINTLN("%s Dispatcher::checkRecv(): unsupported packet version", getLogDateTime());
+  if (len < 0 || !pkt->readFrom(raw, static_cast<size_t>(len))) {
+    MESH_DEBUG_PRINTLN("%s Dispatcher::checkRecv(): partial, corrupt, or unsupported packet received, len=%d", getLogDateTime(), len);
     return false;
   }
-
-  if (pkt->hasTransportCodes()) {
-    memcpy(&pkt->transport_codes[0], &raw[i], 2); i += 2;
-    memcpy(&pkt->transport_codes[1], &raw[i], 2); i += 2;
-  } else {
-    pkt->transport_codes[0] = pkt->transport_codes[1] = 0;
-  }
-
-  pkt->path_len = raw[i++];
-  uint8_t path_mode = pkt->path_len >> 6;  // upper 2 bits (legacy firmware: 00)
-  if (path_mode == 3) {   // Reserved for future
-    MESH_DEBUG_PRINTLN("%s Dispatcher::checkRecv(): unsupported path mode: 3", getLogDateTime());
-    return false;
-  }
-
-  uint8_t path_byte_len = (pkt->path_len & 63) * pkt->getPathHashSize();
-  if (path_byte_len > MAX_PATH_SIZE || i + path_byte_len > len) {
-    MESH_DEBUG_PRINTLN("%s Dispatcher::checkRecv(): partial or corrupt packet received, len=%d", getLogDateTime(), len);
-    return false;
-  }
-
-  memcpy(pkt->path, &raw[i], path_byte_len); i += path_byte_len;
-
-  pkt->payload_len = len - i;  // payload is remainder
-  if (pkt->payload_len > sizeof(pkt->payload)) {
-    MESH_DEBUG_PRINTLN("%s Dispatcher::checkRecv(): packet payload too big, payload_len=%d", getLogDateTime(), (uint32_t)pkt->payload_len);
-    return false;
-  }
-
-  memcpy(pkt->payload, &raw[i], pkt->payload_len);
-
-  return true;  // success
+  return true;
 }
 
 void Dispatcher::checkRecv() {
@@ -387,4 +353,4 @@ unsigned long Dispatcher::futureMillis(int millis_from_now) const {
   return _ms->getMillis() + millis_from_now;
 }
 
-}
+}

src/Mesh.cpp

@@ -158,12 +158,12 @@ DispatcherAction Mesh::onRecvPacket(Packet* pkt) {
             int len = Utils::MACThenDecrypt(secret, data, macAndData, pkt->payload_len - i);
             if (len > 0) {  // success!
               if (pkt->getPayloadType() == PAYLOAD_TYPE_PATH) {
+                if (!Packet::isValidPathPlaintext(data, len)) {
+                  MESH_DEBUG_PRINTLN("%s PAYLOAD_TYPE_PATH, truncated path or missing extra type", getLogDateTime());
+                  break;
+                }
                 int k = 0;
                 uint8_t path_len = data[k++];
-                if (!Packet::isValidPathLen(path_len)) {
-                  MESH_DEBUG_PRINTLN("%s PAYLOAD_TYPE_PATH, bad path_len: %u", getLogDateTime(), (uint32_t)path_len);
-                  break;   // reject bad encoding
-                }
                 uint8_t hash_size = (path_len >> 6) + 1;
                 uint8_t hash_count = path_len & 63;
                 uint8_t* path = &data[k]; k += hash_size*hash_count;
@@ -558,6 +558,11 @@ Packet* Mesh::createGroupDatagram(uint8_t type, const GroupChannel& channel, con
 }
 
 Packet* Mesh::createAck(const uint8_t* ack, uint8_t len) {
+  if (ack == NULL || len < MIN_ACK_PAYLOAD_SIZE || len > MAX_ACK_PAYLOAD_SIZE ||
+      len > sizeof(Packet::payload)) {
+    return NULL;
+  }
+
   Packet* packet = obtainNewPacket();
   if (packet == NULL) {
     MESH_DEBUG_PRINTLN("%s Mesh::createAck(): error, packet pool empty", getLogDateTime());
@@ -572,6 +577,11 @@ Packet* Mesh::createAck(const uint8_t* ack, uint8_t len) {
 }
 
 Packet* Mesh::createMultiAck(const uint8_t* ack, uint8_t len, uint8_t remaining) {
+  if (ack == NULL || len < MIN_ACK_PAYLOAD_SIZE || len > MAX_ACK_PAYLOAD_SIZE ||
+      len > sizeof(Packet::payload) - 1) {
+    return NULL;
+  }
+
   Packet* packet = obtainNewPacket();
   if (packet == NULL) {
     MESH_DEBUG_PRINTLN("%s Mesh::createMultiAck(): error, packet pool empty", getLogDateTime());
@@ -738,4 +748,4 @@ void Mesh::sendZeroHop(Packet* packet, uint16_t* transport_codes, uint32_t delay
   sendPacket(packet, 0, delay_millis);
 }
 
-}
+}

src/MeshCore.h

@@ -10,6 +10,13 @@
 #define SEED_SIZE           32
 #define SIGNATURE_SIZE      64
 #define MAX_ADVERT_DATA_SIZE  32
+
+// ACK payloads always start with the 4-byte acknowledgment hash. Room-server
+// keep-alive ACKs append a 1-byte unsynced-message count (5 bytes total), while
+// extended chat ACKs append an attempt byte and a random byte (6 bytes total).
+#define MIN_ACK_PAYLOAD_SIZE  4
+#define MAX_ACK_PAYLOAD_SIZE  6
+
 #define CIPHER_KEY_SIZE     16
 #define CIPHER_BLOCK_SIZE   16
 

src/Packet.cpp

@@ -4,6 +4,60 @@
 
 namespace mesh {
 
+namespace {
+
+bool hasCompleteCiphertext(size_t payload_len, size_t prefix_len) {
+  const size_t overhead = prefix_len + CIPHER_MAC_SIZE;
+  return payload_len >= overhead + CIPHER_BLOCK_SIZE &&
+         (payload_len - overhead) % CIPHER_BLOCK_SIZE == 0;
+}
+
+bool isValidPayload(uint8_t header, uint8_t path_len, const uint8_t* payload, size_t payload_len) {
+  const uint8_t type = (header >> PH_TYPE_SHIFT) & PH_TYPE_MASK;
+  switch (type) {
+    case PAYLOAD_TYPE_REQ:
+    case PAYLOAD_TYPE_RESPONSE:
+    case PAYLOAD_TYPE_TXT_MSG:
+    case PAYLOAD_TYPE_PATH:
+      return hasCompleteCiphertext(payload_len, 2);
+    case PAYLOAD_TYPE_ACK:
+      return payload_len >= MIN_ACK_PAYLOAD_SIZE && payload_len <= MAX_ACK_PAYLOAD_SIZE;
+    case PAYLOAD_TYPE_ADVERT:
+      return payload_len >= PUB_KEY_SIZE + sizeof(uint32_t) + SIGNATURE_SIZE &&
+             payload_len <= PUB_KEY_SIZE + sizeof(uint32_t) + SIGNATURE_SIZE + MAX_ADVERT_DATA_SIZE;
+    case PAYLOAD_TYPE_GRP_TXT:
+    case PAYLOAD_TYPE_GRP_DATA:
+      return hasCompleteCiphertext(payload_len, 1);
+    case PAYLOAD_TYPE_ANON_REQ:
+      return hasCompleteCiphertext(payload_len, 1 + PUB_KEY_SIZE);
+    case PAYLOAD_TYPE_TRACE: {
+      if ((path_len & 0xc0) != 0 || payload_len < 9) return false;
+      const uint8_t flags = payload[8];
+      const size_t hash_size = 1u << (flags & 0x03);
+      return (flags & 0xfc) == 0 && (payload_len - 9) % hash_size == 0;
+    }
+    case PAYLOAD_TYPE_MULTIPART:
+      if (payload_len == 0) return false;
+      if ((payload[0] & 0x0f) == PAYLOAD_TYPE_ACK) {
+        const size_t ack_len = payload_len - 1;
+        return ack_len >= MIN_ACK_PAYLOAD_SIZE && ack_len <= MAX_ACK_PAYLOAD_SIZE;
+      }
+      return true;
+    case PAYLOAD_TYPE_CONTROL:
+      if (payload_len == 0) return false;
+      if (type == PAYLOAD_TYPE_CONTROL && (payload[0] & 0x80) != 0) {
+        const uint8_t route = header & PH_ROUTE_MASK;
+        return (route == ROUTE_TYPE_DIRECT || route == ROUTE_TYPE_TRANSPORT_DIRECT) &&
+               (path_len & 0x3f) == 0;
+      }
+      return true;
+    default:
+      return true;
+  }
+}
+
+}  // namespace
+
 Packet::Packet() {
   header = 0;
   path_len = 0;
@@ -17,6 +71,18 @@ bool Packet::isValidPathLen(uint8_t path_len) {
   return hash_count*hash_size <= MAX_PATH_SIZE;
 }
 
+bool Packet::hasCompletePath(const uint8_t* data, size_t len) {
+  if (data == nullptr || len == 0 || !isValidPathLen(data[0])) return false;
+  const size_t path_byte_len = (data[0] & 63) * ((data[0] >> 6) + 1);
+  return path_byte_len <= len - 1;
+}
+
+bool Packet::isValidPathPlaintext(const uint8_t* data, size_t len) {
+  if (len < 2 || !hasCompletePath(data, len)) return false;
+  const size_t path_byte_len = (data[0] & 63) * ((data[0] >> 6) + 1);
+  return path_byte_len < len - 1;  // One byte after the path is required for extra_type.
+}
+
 size_t Packet::writePath(uint8_t* dest, const uint8_t* src, uint8_t path_len) {
   uint8_t hash_count = path_len & 63;
   uint8_t hash_size = (path_len >> 6) + 1;
@@ -62,26 +128,49 @@ uint8_t Packet::writeTo(uint8_t dest[]) const {
   return i;
 }
 
-bool Packet::readFrom(const uint8_t src[], uint8_t len) {
-  uint8_t i = 0;
-  header = src[i++];
-  if (hasTransportCodes()) {
-    memcpy(&transport_codes[0], &src[i], 2); i += 2;
-    memcpy(&transport_codes[1], &src[i], 2); i += 2;
-  } else {
-    transport_codes[0] = transport_codes[1] = 0;
+bool Packet::readFrom(const uint8_t src[], size_t len) {
+  if (src == nullptr || len == 0 || len > MAX_TRANS_UNIT) return false;
+
+  size_t i = 0;
+  const uint8_t decoded_header = src[i++];
+  if (((decoded_header >> PH_VER_SHIFT) & PH_VER_MASK) > PAYLOAD_VER_1) return false;
+
+  uint16_t decoded_transport_codes[2] = {};
+  const bool has_transport_codes =
+      (decoded_header & PH_ROUTE_MASK) == ROUTE_TYPE_TRANSPORT_FLOOD ||
+      (decoded_header & PH_ROUTE_MASK) == ROUTE_TYPE_TRANSPORT_DIRECT;
+  if (has_transport_codes) {
+    if (len - i < sizeof(decoded_transport_codes)) return false;
+    memcpy(decoded_transport_codes, &src[i], sizeof(decoded_transport_codes));
+    i += sizeof(decoded_transport_codes);
   }
-  path_len = src[i++];
-  if (!isValidPathLen(path_len)) return false;   // bad encoding
 
-  uint8_t bl = getPathByteLen();
-  memcpy(path, &src[i], bl); i += bl;
+  if (i >= len) return false;
+  const uint8_t decoded_path_len = src[i++];
+  if (!isValidPathLen(decoded_path_len)) return false;
 
-  if (i >= len) return false;   // bad encoding
-  payload_len = len - i;
-  if (payload_len > sizeof(payload)) return false;  // bad encoding
-  memcpy(payload, &src[i], payload_len); //i += payload_len;
+  const uint8_t hash_count = decoded_path_len & 63;
+  const uint8_t hash_size = (decoded_path_len >> 6) + 1;
+  const size_t path_byte_len = hash_count * hash_size;
+  if (len - i < path_byte_len) return false;
+
+  const size_t decoded_payload_len = len - i - path_byte_len;
+  if (decoded_payload_len > sizeof(payload)) return false;
+
+  const uint8_t* decoded_payload = &src[i + path_byte_len];
+  if (!isValidPayload(decoded_header, decoded_path_len, decoded_payload, decoded_payload_len)) {
+    return false;
+  }
+
+  header = decoded_header;
+  transport_codes[0] = decoded_transport_codes[0];
+  transport_codes[1] = decoded_transport_codes[1];
+  path_len = decoded_path_len;
+  memcpy(path, &src[i], path_byte_len);
+  i += path_byte_len;
+  payload_len = decoded_payload_len;
+  memcpy(payload, decoded_payload, payload_len);
   return true;   // success
 }
 
-}
+}

src/Packet.h

@@ -85,6 +85,8 @@ class Packet {
   static uint8_t copyPath(uint8_t* dest, const uint8_t* src, uint8_t path_len);  // returns path_len
   static size_t writePath(uint8_t* dest, const uint8_t* src, uint8_t path_len);  // returns byte length written
   static bool isValidPathLen(uint8_t path_len);
+  static bool hasCompletePath(const uint8_t* data, size_t len);
+  static bool isValidPathPlaintext(const uint8_t* data, size_t len);
 
   void markDoNotRetransmit() { header = 0xFF; }
   bool isMarkedDoNotRetransmit() const { return header == 0xFF; }
@@ -108,7 +110,7 @@ class Packet {
    * \param  src  (IN) buffer containing blob
    * \param  len  the packet length (as returned by writeTo())
    */
-  bool readFrom(const uint8_t src[], uint8_t len);
+  bool readFrom(const uint8_t src[], size_t len);
 };
 
 }

src/Utils.cpp

@@ -51,6 +51,7 @@ void Utils::sha256(uint8_t *hash, size_t hash_len, const uint8_t* frag1, int fra
 }
 
 int Utils::decrypt(const uint8_t* shared_secret, uint8_t* dest, const uint8_t* src, int src_len) {
+  if (src_len <= 0 || src_len % CIPHER_BLOCK_SIZE != 0) return 0;
 #ifdef USE_CC310_HW_CRYPTO
   static SaSiAesUserContext_t ctx;
   SaSiAesUserKeyData_t keyData = { (uint8_t*)shared_secret, CIPHER_KEY_SIZE };