Skip to content

Commit 4e33c09

Browse files
committed
Bounds-check reply_path in anonymous request handlers
The handleAnon*Req functions read a reply_path_len byte from the decrypted data and memcpy that many bytes into reply_path, without checking that the data buffer actually contains that many bytes. With a minimal-length packet, this reads up to 63 bytes of uninitialized stack memory. Add a data_len parameter to all three handlers and validate that the buffer contains enough bytes for the claimed reply_path_len before copying. Also guard the callers to ensure len > 5 before passing &data[5].
1 parent e33d93d commit 4e33c09

2 files changed

Lines changed: 18 additions & 12 deletions

File tree

examples/simple_repeater/MyMesh.cpp

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -144,10 +144,12 @@ uint8_t MyMesh::handleLoginReq(const mesh::Identity& sender, const uint8_t* secr
144144
return 13; // reply length
145145
}
146146

147-
uint8_t MyMesh::handleAnonRegionsReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data) {
147+
uint8_t MyMesh::handleAnonRegionsReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data, size_t data_len) {
148148
if (anon_limiter.allow(rtc_clock.getCurrentTime())) {
149149
// request data has: {reply-path-len}{reply-path}
150+
if (data_len < 1) return 0;
150151
reply_path_len = *data++ & 0x3F;
152+
if (1 + reply_path_len > data_len) return 0;
151153
memcpy(reply_path, data, reply_path_len);
152154
// data += reply_path_len;
153155

@@ -160,10 +162,12 @@ uint8_t MyMesh::handleAnonRegionsReq(const mesh::Identity& sender, uint32_t send
160162
return 0;
161163
}
162164

163-
uint8_t MyMesh::handleAnonOwnerReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data) {
165+
uint8_t MyMesh::handleAnonOwnerReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data, size_t data_len) {
164166
if (anon_limiter.allow(rtc_clock.getCurrentTime())) {
165167
// request data has: {reply-path-len}{reply-path}
168+
if (data_len < 1) return 0;
166169
reply_path_len = *data++ & 0x3F;
170+
if (1 + reply_path_len > data_len) return 0;
167171
memcpy(reply_path, data, reply_path_len);
168172
// data += reply_path_len;
169173

@@ -177,10 +181,12 @@ uint8_t MyMesh::handleAnonOwnerReq(const mesh::Identity& sender, uint32_t sender
177181
return 0;
178182
}
179183

180-
uint8_t MyMesh::handleAnonClockReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data) {
184+
uint8_t MyMesh::handleAnonClockReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data, size_t data_len) {
181185
if (anon_limiter.allow(rtc_clock.getCurrentTime())) {
182186
// request data has: {reply-path-len}{reply-path}
187+
if (data_len < 1) return 0;
183188
reply_path_len = *data++ & 0x3F;
189+
if (1 + reply_path_len > data_len) return 0;
184190
memcpy(reply_path, data, reply_path_len);
185191
// data += reply_path_len;
186192

@@ -518,12 +524,12 @@ void MyMesh::onAnonDataRecv(mesh::Packet *packet, const uint8_t *secret, const m
518524
reply_path_len = -1;
519525
if (data[4] == 0 || data[4] >= ' ') { // is password, ie. a login request
520526
reply_len = handleLoginReq(sender, secret, timestamp, &data[4], packet->isRouteFlood());
521-
} else if (data[4] == ANON_REQ_TYPE_REGIONS && packet->isRouteDirect()) {
522-
reply_len = handleAnonRegionsReq(sender, timestamp, &data[5]);
523-
} else if (data[4] == ANON_REQ_TYPE_OWNER && packet->isRouteDirect()) {
524-
reply_len = handleAnonOwnerReq(sender, timestamp, &data[5]);
525-
} else if (data[4] == ANON_REQ_TYPE_BASIC && packet->isRouteDirect()) {
526-
reply_len = handleAnonClockReq(sender, timestamp, &data[5]);
527+
} else if (data[4] == ANON_REQ_TYPE_REGIONS && packet->isRouteDirect() && len > 5) {
528+
reply_len = handleAnonRegionsReq(sender, timestamp, &data[5], len - 5);
529+
} else if (data[4] == ANON_REQ_TYPE_OWNER && packet->isRouteDirect() && len > 5) {
530+
reply_len = handleAnonOwnerReq(sender, timestamp, &data[5], len - 5);
531+
} else if (data[4] == ANON_REQ_TYPE_BASIC && packet->isRouteDirect() && len > 5) {
532+
reply_len = handleAnonClockReq(sender, timestamp, &data[5], len - 5);
527533
} else {
528534
reply_len = 0; // unknown/invalid request type
529535
}

examples/simple_repeater/MyMesh.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,9 @@ class MyMesh : public mesh::Mesh, public CommonCLICallbacks {
117117

118118
void putNeighbour(const mesh::Identity& id, uint32_t timestamp, float snr);
119119
uint8_t handleLoginReq(const mesh::Identity& sender, const uint8_t* secret, uint32_t sender_timestamp, const uint8_t* data, bool is_flood);
120-
uint8_t handleAnonRegionsReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data);
121-
uint8_t handleAnonOwnerReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data);
122-
uint8_t handleAnonClockReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data);
120+
uint8_t handleAnonRegionsReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data, size_t data_len);
121+
uint8_t handleAnonOwnerReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data, size_t data_len);
122+
uint8_t handleAnonClockReq(const mesh::Identity& sender, uint32_t sender_timestamp, const uint8_t* data, size_t data_len);
123123
int handleRequest(ClientInfo* sender, uint32_t sender_timestamp, uint8_t* payload, size_t payload_len);
124124
mesh::Packet* createSelfAdvert();
125125

0 commit comments

Comments
 (0)