Skip to content

Commit

Permalink
Merge pull request #394 from embhorn/zd17257
Browse files Browse the repository at this point in the history 
Fix string prop OOB read
  • Loading branch information
@philljj
philljj authored Feb 23, 2024
2 parents ff496d6 + d9f6be8 commit c923292
Showing 1 changed file with 36 additions and 5 deletions.
41 changes: 36 additions & 5 deletions src/mqtt_packet.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,10 @@ int MqttDecode_Props(MqttPacketType packet, MqttProp** props, byte* pbuf,
}

/* Decode the Identifier */
if (buf_len < (word32)(buf - pbuf)) {
rc = MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
break;
}
rc = MqttDecode_Vbi(buf, (word32*)&cur_prop->type,
(word32)(buf_len - (buf - pbuf)));
if (rc < 0) {
Expand Down Expand Up @@ -549,7 +553,7 @@ int MqttDecode_Props(MqttPacketType packet, MqttProp** props, byte* pbuf,
tmp = MqttDecode_String(buf,
(const char**)&cur_prop->data_str.str,
&cur_prop->data_str.len);
if (cur_prop->data_str.len <= (buf_len - (buf - pbuf))) {
if ((tmp >= 0) && ((word32)tmp <= (buf_len - (buf - pbuf)))) {
buf += tmp;
total += tmp;
prop_len -= (word32)tmp;
Expand All @@ -562,6 +566,10 @@ int MqttDecode_Props(MqttPacketType packet, MqttProp** props, byte* pbuf,
}
case MQTT_DATA_TYPE_VAR_INT:
{
if (buf_len < (word32)(buf - pbuf)) {
rc = MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
break;
}
tmp = MqttDecode_Vbi(buf, &cur_prop->data_int,
(word32)(buf_len - (buf - pbuf)));
if (tmp < 0) {
Expand Down Expand Up @@ -600,17 +608,16 @@ int MqttDecode_Props(MqttPacketType packet, MqttProp** props, byte* pbuf,
tmp = MqttDecode_String(buf,
(const char**)&cur_prop->data_str.str,
&cur_prop->data_str.len);
if (cur_prop->data_str.len <=
(buf_len - (buf - pbuf))) {
if ((tmp >= 0) && ((word32)tmp <= (buf_len - (buf - pbuf)))) {
buf += tmp;
total += tmp;
prop_len -= (word32)tmp;
if ((buf_len - (buf - pbuf)) > 0) {
tmp = MqttDecode_String(buf,
(const char**)&cur_prop->data_str2.str,
&cur_prop->data_str2.len);
if (cur_prop->data_str2.len <=
(buf_len - (buf - pbuf))) {
if ((tmp >= 0) && ((word32)tmp <=
(buf_len - (buf - pbuf)))) {
buf += tmp;
total += tmp;
prop_len -= (word32)tmp;
Expand Down Expand Up @@ -830,6 +837,9 @@ int MqttDecode_ConnectAck(byte *rx_buf, int rx_buf_len,
word32 props_len = 0;
int tmp;
/* Decode Length of Properties */
if (rx_buf_len < (rx_payload - rx_buf)) {
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
}
tmp = MqttDecode_Vbi(rx_payload, &props_len,
(word32)(rx_buf_len - (rx_payload - rx_buf)));
if (tmp < 0) {
Expand Down Expand Up @@ -1012,6 +1022,9 @@ int MqttDecode_Publish(byte *rx_buf, int rx_buf_len, MqttPublish *publish)
int tmp;

/* Decode Length of Properties */
if (rx_buf_len < (rx_payload - rx_buf)) {
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
}
tmp = MqttDecode_Vbi(rx_payload, &props_len,
(word32)(rx_buf_len - (rx_payload - rx_buf)));
if (tmp < 0)
Expand Down Expand Up @@ -1170,6 +1183,9 @@ int MqttDecode_PublishResp(byte* rx_buf, int rx_buf_len, byte type,
int tmp;

/* Decode Length of Properties */
if (rx_buf_len < (rx_payload - rx_buf)) {
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
}
tmp = MqttDecode_Vbi(rx_payload, &props_len,
(word32)(rx_buf_len - (rx_payload - rx_buf)));
if (tmp < 0)
Expand Down Expand Up @@ -1314,6 +1330,9 @@ int MqttDecode_SubscribeAck(byte* rx_buf, int rx_buf_len,
int tmp;

/* Decode Length of Properties */
if (rx_buf_len < (rx_payload - rx_buf)) {
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
}
tmp = MqttDecode_Vbi(rx_payload, &props_len,
(word32)(rx_buf_len - (rx_payload - rx_buf)));
if (tmp < 0)
Expand Down Expand Up @@ -1453,6 +1472,9 @@ int MqttDecode_UnsubscribeAck(byte *rx_buf, int rx_buf_len,
int tmp;

/* Decode Length of Properties */
if (rx_buf_len < (rx_payload - rx_buf)) {
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
}
tmp = MqttDecode_Vbi(rx_payload, &props_len,
(word32)(rx_buf_len - (rx_payload - rx_buf)));
if (tmp < 0)
Expand Down Expand Up @@ -1636,6 +1658,9 @@ int MqttDecode_Disconnect(byte *rx_buf, int rx_buf_len, MqttDisconnect *disc)

if (remain_len > 1) {
/* Decode Length of Properties */
if (rx_buf_len < (rx_payload - rx_buf)) {
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
}
tmp = MqttDecode_Vbi(rx_payload, &props_len,
(word32)(rx_buf_len - (rx_payload - rx_buf)));
if (tmp < 0)
Expand Down Expand Up @@ -1751,6 +1776,9 @@ int MqttDecode_Auth(byte *rx_buf, int rx_buf_len, MqttAuth *auth)
auth->props = NULL;

/* Decode Length of Properties */
if (rx_buf_len < (rx_payload - rx_buf)) {
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
}
tmp = MqttDecode_Vbi(rx_payload, &props_len,
(word32)(rx_buf_len - (rx_payload - rx_buf)));
if (tmp < 0)
Expand Down Expand Up @@ -2019,6 +2047,9 @@ int MqttPacket_Read(MqttClient *client, byte* rx_buf, int rx_buf_len,
}

/* Try and decode remaining length */
if (rx_buf_len < (client->packet.header_len - (i + 1))) {
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
}
rc = MqttDecode_Vbi(header->len,
(word32*)&client->packet.remain_len,
rx_buf_len - (client->packet.header_len - (i + 1)));
Expand Down

0 comments on commit c923292

Please sign in to comment.