Skip to content

Commit 0884338

Browse files
author
robert
committed
Improve mqtt props bound checks
1 parent ba76869 commit 0884338

File tree

2 files changed

+22
-16
lines changed

2 files changed

+22
-16
lines changed

‎mongoose.c‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4111,10 +4111,12 @@ size_t mg_mqtt_next_prop(struct mg_mqtt_message *msg, struct mg_mqtt_prop *prop,
41114111
uint8_t *i = (uint8_t *) msg->dgram.buf + msg->props_start + ofs;
41124112
uint8_t *end = (uint8_t *) msg->dgram.buf + msg->dgram.len;
41134113
size_t new_pos = ofs, len;
4114-
prop->id = i[0];
41154114

41164115
if (ofs >= msg->dgram.len || ofs >= msg->props_start + msg->props_size || (i + 1) >= end)
41174116
return 0;
4117+
4118+
memset(prop, 0, sizeof(struct mg_mqtt_prop));
4119+
prop->id = i[0];
41184120
i++, new_pos++;
41194121

41204122
switch (mqtt_prop_type_by_id(prop->id)) {
@@ -4126,21 +4128,21 @@ size_t mg_mqtt_next_prop(struct mg_mqtt_message *msg, struct mg_mqtt_prop *prop,
41264128
if (i + 2 >= end) return 0;
41274129
prop->val.len = (uint16_t) ((((uint16_t) i[0]) << 8) | i[1]);
41284130
prop->val.buf = (char *) i + 2;
4129-
if (i + 2 + prop->val.len > end) return 0;
4131+
if (i + 2 + prop->val.len >= end) return 0;
41304132
new_pos += 2 * sizeof(uint16_t) + prop->val.len + prop->key.len;
41314133
break;
41324134
case MQTT_PROP_TYPE_BYTE:
4133-
if (i + 1 > end) return 0;
4135+
if (i + 1 >= end) return 0;
41344136
prop->iv = (uint8_t) i[0];
41354137
new_pos++;
41364138
break;
41374139
case MQTT_PROP_TYPE_SHORT:
4138-
if (i + 2 > end) return 0;
4140+
if (i + 2 >= end) return 0;
41394141
prop->iv = (uint16_t) ((((uint16_t) i[0]) << 8) | i[1]);
41404142
new_pos += sizeof(uint16_t);
41414143
break;
41424144
case MQTT_PROP_TYPE_INT:
4143-
if (i + 4 > end) return 0;
4145+
if (i + 4 >= end) return 0;
41444146
prop->iv = ((uint32_t) i[0] << 24) | ((uint32_t) i[1] << 16) |
41454147
((uint32_t) i[2] << 8) | i[3];
41464148
new_pos += sizeof(uint32_t);
@@ -4149,18 +4151,19 @@ size_t mg_mqtt_next_prop(struct mg_mqtt_message *msg, struct mg_mqtt_prop *prop,
41494151
if (i + 2 >= end) return 0;
41504152
prop->val.len = (uint16_t) ((((uint16_t) i[0]) << 8) | i[1]);
41514153
prop->val.buf = (char *) i + 2;
4152-
if (i + 2 + prop->val.len > end) return 0;
4154+
if (i + 2 + prop->val.len >= end) return 0;
41534155
new_pos += 2 + prop->val.len;
41544156
break;
41554157
case MQTT_PROP_TYPE_BINARY_DATA:
4156-
if (i + 2 > end) return 0;
4158+
if (i + 2 >= end) return 0;
41574159
prop->val.len = (uint16_t) ((((uint16_t) i[0]) << 8) | i[1]);
41584160
prop->val.buf = (char *) i + 2;
4161+
if (i + 2 + prop->val.len >= end) return 0;
41594162
new_pos += 2 + prop->val.len;
41604163
break;
41614164
case MQTT_PROP_TYPE_VARIABLE_INT:
41624165
len = decode_varint(i, (size_t) (end - i), (size_t *) &prop->iv);
4163-
if (i + len > end) return 0;
4166+
if (i + len >= end) return 0;
41644167
new_pos = (len == 0) ? 0 : new_pos + len;
41654168
break;
41664169
default:

‎src/mqtt.c‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -206,10 +206,12 @@ size_t mg_mqtt_next_prop(struct mg_mqtt_message *msg, struct mg_mqtt_prop *prop,
206206
uint8_t *i = (uint8_t *) msg->dgram.buf + msg->props_start + ofs;
207207
uint8_t *end = (uint8_t *) msg->dgram.buf + msg->dgram.len;
208208
size_t new_pos = ofs, len;
209-
prop->id = i[0];
210209

211210
if (ofs >= msg->dgram.len || ofs >= msg->props_start + msg->props_size || (i + 1) >= end)
212211
return 0;
212+
213+
memset(prop, 0, sizeof(struct mg_mqtt_prop));
214+
prop->id = i[0];
213215
i++, new_pos++;
214216

215217
switch (mqtt_prop_type_by_id(prop->id)) {
@@ -221,21 +223,21 @@ size_t mg_mqtt_next_prop(struct mg_mqtt_message *msg, struct mg_mqtt_prop *prop,
221223
if (i + 2 >= end) return 0;
222224
prop->val.len = (uint16_t) ((((uint16_t) i[0]) << 8) | i[1]);
223225
prop->val.buf = (char *) i + 2;
224-
if (i + 2 + prop->val.len > end) return 0;
226+
if (i + 2 + prop->val.len >= end) return 0;
225227
new_pos += 2 * sizeof(uint16_t) + prop->val.len + prop->key.len;
226228
break;
227229
case MQTT_PROP_TYPE_BYTE:
228-
if (i + 1 > end) return 0;
230+
if (i + 1 >= end) return 0;
229231
prop->iv = (uint8_t) i[0];
230232
new_pos++;
231233
break;
232234
case MQTT_PROP_TYPE_SHORT:
233-
if (i + 2 > end) return 0;
235+
if (i + 2 >= end) return 0;
234236
prop->iv = (uint16_t) ((((uint16_t) i[0]) << 8) | i[1]);
235237
new_pos += sizeof(uint16_t);
236238
break;
237239
case MQTT_PROP_TYPE_INT:
238-
if (i + 4 > end) return 0;
240+
if (i + 4 >= end) return 0;
239241
prop->iv = ((uint32_t) i[0] << 24) | ((uint32_t) i[1] << 16) |
240242
((uint32_t) i[2] << 8) | i[3];
241243
new_pos += sizeof(uint32_t);
@@ -244,18 +246,19 @@ size_t mg_mqtt_next_prop(struct mg_mqtt_message *msg, struct mg_mqtt_prop *prop,
244246
if (i + 2 >= end) return 0;
245247
prop->val.len = (uint16_t) ((((uint16_t) i[0]) << 8) | i[1]);
246248
prop->val.buf = (char *) i + 2;
247-
if (i + 2 + prop->val.len > end) return 0;
249+
if (i + 2 + prop->val.len >= end) return 0;
248250
new_pos += 2 + prop->val.len;
249251
break;
250252
case MQTT_PROP_TYPE_BINARY_DATA:
251-
if (i + 2 > end) return 0;
253+
if (i + 2 >= end) return 0;
252254
prop->val.len = (uint16_t) ((((uint16_t) i[0]) << 8) | i[1]);
253255
prop->val.buf = (char *) i + 2;
256+
if (i + 2 + prop->val.len >= end) return 0;
254257
new_pos += 2 + prop->val.len;
255258
break;
256259
case MQTT_PROP_TYPE_VARIABLE_INT:
257260
len = decode_varint(i, (size_t) (end - i), (size_t *) &prop->iv);
258-
if (i + len > end) return 0;
261+
if (i + len >= end) return 0;
259262
new_pos = (len == 0) ? 0 : new_pos + len;
260263
break;
261264
default:

0 commit comments

Comments
 (0)