Skip to content

Commit a98ef95

Browse files
kareem-wolfsslkareem-wolfssl
committed
Always check MqttDecode_Num's return code.
Thanks to Weiheng Qiu for the report.
1 parent 5873c5e commit a98ef95

1 file changed

Lines changed: 124 additions & 22 deletions

File tree

‎src/mqtt_sn_packet.c‎

Lines changed: 124 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ const char* SN_Packet_TypeDesc(SN_MsgType packet_type)
9898
int SN_Decode_Header(byte *rx_buf, int rx_buf_len,
9999
SN_MsgType* p_packet_type, word16* p_packet_id)
100100
{
101+
int rc;
101102
SN_MsgType packet_type;
102103
word16 total_len;
103104

@@ -109,7 +110,11 @@ int SN_Decode_Header(byte *rx_buf, int rx_buf_len,
109110
total_len = *rx_buf++;
110111
if (total_len == SN_PACKET_LEN_IND) {
111112
/* The length is stored in the next two bytes */
112-
rx_buf += MqttDecode_Num(rx_buf, &total_len, rx_buf_len - 1);
113+
rc = MqttDecode_Num(rx_buf, &total_len, rx_buf_len - 1);
114+
if (rc < 0) {
115+
return rc;
116+
}
117+
rx_buf += rc;
113118
}
114119

115120
if (total_len > rx_buf_len) {
@@ -126,19 +131,37 @@ int SN_Decode_Header(byte *rx_buf, int rx_buf_len,
126131
switch(packet_type) {
127132
case SN_MSG_TYPE_REGACK:
128133
case SN_MSG_TYPE_PUBACK:
134+
if (rx_buf_len < 3) {
135+
MQTT_TRACE_ERROR(MQTT_CODE_ERROR_BAD_ARG);
136+
}
129137
/* octet 4-5 */
130-
MqttDecode_Num(rx_buf + 2, p_packet_id, (word32)(rx_buf_len - 3));
138+
rc = MqttDecode_Num(rx_buf + 2, p_packet_id,
139+
(word32)(rx_buf_len - 3));
140+
if (rc < 0) {
141+
return rc;
142+
}
131143
break;
132144
case SN_MSG_TYPE_PUBCOMP:
133145
case SN_MSG_TYPE_PUBREC:
134146
case SN_MSG_TYPE_PUBREL:
135147
case SN_MSG_TYPE_UNSUBACK:
136148
/* octet 2-3 */
137-
MqttDecode_Num(rx_buf, p_packet_id, (word32)(rx_buf_len - 1));
149+
rc = MqttDecode_Num(rx_buf, p_packet_id,
150+
(word32)(rx_buf_len - 1));
151+
if (rc < 0) {
152+
return rc;
153+
}
138154
break;
139155
case SN_MSG_TYPE_SUBACK:
156+
if (rx_buf_len < 4) {
157+
MQTT_TRACE_ERROR(MQTT_CODE_ERROR_BAD_ARG);
158+
}
140159
/* octet 5-6 */
141-
MqttDecode_Num(rx_buf + 3, p_packet_id, (word32)(rx_buf_len - 4));
160+
rc = MqttDecode_Num(rx_buf + 3, p_packet_id,
161+
(word32)(rx_buf_len - 4));
162+
if (rc < 0) {
163+
return rc;
164+
}
142165
break;
143166
case SN_MSG_TYPE_ADVERTISE:
144167
case SN_MSG_TYPE_SEARCHGW:
@@ -199,9 +222,15 @@ int SN_Decode_Advertise(byte *rx_buf, int rx_buf_len, SN_Advertise *gw_info)
199222

200223
/* Decode gateway info */
201224
if (gw_info != NULL) {
225+
int rc;
202226
gw_info->gwId = *rx_payload++;
203227

204-
rx_payload += MqttDecode_Num(rx_payload, &gw_info->duration, (word32)(rx_buf_len - (rx_payload - rx_buf)));
228+
rc = MqttDecode_Num(rx_payload, &gw_info->duration,
229+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
230+
if (rc < 0) {
231+
return rc;
232+
}
233+
rx_payload += rc;
205234
}
206235
(void)rx_payload;
207236

@@ -239,6 +268,7 @@ int SN_Encode_SearchGW(byte *tx_buf, int tx_buf_len, byte hops)
239268
int SN_Decode_GWInfo(byte *rx_buf, int rx_buf_len, SN_GwInfo *gw_info)
240269
{
241270
word16 total_len;
271+
int rc;
242272
byte *rx_payload = rx_buf, type;
243273

244274
/* Validate required arguments */
@@ -250,7 +280,12 @@ int SN_Decode_GWInfo(byte *rx_buf, int rx_buf_len, SN_GwInfo *gw_info)
250280
total_len = *rx_payload++;
251281
if (total_len == SN_PACKET_LEN_IND) {
252282
/* The length is stored in the next two bytes */
253-
rx_payload += MqttDecode_Num(rx_payload, &total_len, (word32)(rx_buf_len - (rx_payload - rx_buf)));
283+
rc = MqttDecode_Num(rx_payload, &total_len,
284+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
285+
if (rc < 0) {
286+
return rc;
287+
}
288+
rx_payload += rc;
254289
}
255290

256291
if (total_len > rx_buf_len) {
@@ -790,6 +825,7 @@ int SN_Encode_Register(byte *tx_buf, int tx_buf_len, SN_Register *regist)
790825
int SN_Decode_Register(byte *rx_buf, int rx_buf_len, SN_Register *regist)
791826
{
792827
word16 total_len;
828+
int rc;
793829
byte *rx_payload = rx_buf, type;
794830

795831
/* Validate required arguments */
@@ -801,7 +837,12 @@ int SN_Decode_Register(byte *rx_buf, int rx_buf_len, SN_Register *regist)
801837
total_len = *rx_payload++;
802838
if (total_len == SN_PACKET_LEN_IND) {
803839
/* The length is stored in the next two bytes */
804-
rx_payload += MqttDecode_Num(rx_payload, &total_len, (word32)(rx_buf_len - (rx_payload - rx_buf)));
840+
rc = MqttDecode_Num(rx_payload, &total_len,
841+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
842+
if (rc < 0) {
843+
return rc;
844+
}
845+
rx_payload += rc;
805846
}
806847

807848
if (total_len >= rx_buf_len) {
@@ -819,10 +860,20 @@ int SN_Decode_Register(byte *rx_buf, int rx_buf_len, SN_Register *regist)
819860

820861
if (regist != NULL) {
821862
/* Decode Topic ID assigned by GW */
822-
rx_payload += MqttDecode_Num(rx_payload, &regist->topicId, (word32)(rx_buf_len - (rx_payload - rx_buf)));
863+
rc = MqttDecode_Num(rx_payload, &regist->topicId,
864+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
865+
if (rc < 0) {
866+
return rc;
867+
}
868+
rx_payload += rc;
823869

824870
/* Decode packet ID */
825-
rx_payload += MqttDecode_Num(rx_payload, &regist->packet_id, (word32)(rx_buf_len - (rx_payload - rx_buf)));
871+
rc = MqttDecode_Num(rx_payload, &regist->packet_id,
872+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
873+
if (rc < 0) {
874+
return rc;
875+
}
876+
rx_payload += rc;
826877

827878
/* Decode Topic Name */
828879
regist->topicName = (char*)rx_payload;
@@ -879,7 +930,7 @@ int SN_Encode_RegAck(byte *tx_buf, int tx_buf_len, SN_RegAck *regack)
879930

880931
int SN_Decode_RegAck(byte *rx_buf, int rx_buf_len, SN_RegAck *regack)
881932
{
882-
int total_len;
933+
int rc, total_len;
883934
byte *rx_payload = rx_buf, type;
884935

885936
/* Validate required arguments */
@@ -904,10 +955,20 @@ int SN_Decode_RegAck(byte *rx_buf, int rx_buf_len, SN_RegAck *regack)
904955

905956
if (regack != NULL) {
906957
/* Decode Topic ID assigned by GW */
907-
rx_payload += MqttDecode_Num(rx_payload, &regack->topicId, (word32)(rx_buf_len - (rx_payload - rx_buf)));
958+
rc = MqttDecode_Num(rx_payload, &regack->topicId,
959+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
960+
if (rc < 0) {
961+
return rc;
962+
}
963+
rx_payload += rc;
908964

909965
/* Decode packet ID */
910-
rx_payload += MqttDecode_Num(rx_payload, &regack->packet_id, (word32)(rx_buf_len - (rx_payload - rx_buf)));
966+
rc = MqttDecode_Num(rx_payload, &regack->packet_id,
967+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
968+
if (rc < 0) {
969+
return rc;
970+
}
971+
rx_payload += rc;
911972

912973
/* Decode return code */
913974
regack->return_code = *rx_payload++;
@@ -994,6 +1055,7 @@ int SN_Encode_Subscribe(byte *tx_buf, int tx_buf_len, SN_Subscribe *subscribe)
9941055
int SN_Decode_SubscribeAck(byte* rx_buf, int rx_buf_len,
9951056
SN_SubAck *subscribe_ack)
9961057
{
1058+
int rc;
9971059
word16 total_len;
9981060
byte* rx_payload = rx_buf, type;
9991061

@@ -1020,8 +1082,18 @@ int SN_Decode_SubscribeAck(byte* rx_buf, int rx_buf_len,
10201082
/* Decode SubAck fields */
10211083
if (subscribe_ack) {
10221084
subscribe_ack->flags = *rx_payload++;
1023-
rx_payload += MqttDecode_Num(rx_payload, &subscribe_ack->topicId, (word32)(rx_buf_len - (rx_payload - rx_buf)));
1024-
rx_payload += MqttDecode_Num(rx_payload, &subscribe_ack->packet_id, (word32)(rx_buf_len - (rx_payload - rx_buf)));
1085+
rc = MqttDecode_Num(rx_payload, &subscribe_ack->topicId,
1086+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
1087+
if (rc < 0) {
1088+
return rc;
1089+
}
1090+
rx_payload += rc;
1091+
rc = MqttDecode_Num(rx_payload, &subscribe_ack->packet_id,
1092+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
1093+
if (rc < 0) {
1094+
return rc;
1095+
}
1096+
rx_payload += rc;
10251097
subscribe_ack->return_code = *rx_payload++;
10261098
}
10271099
(void)rx_payload;
@@ -1112,6 +1184,7 @@ int SN_Encode_Publish(byte *tx_buf, int tx_buf_len, SN_Publish *publish)
11121184

11131185
int SN_Decode_Publish(byte *rx_buf, int rx_buf_len, SN_Publish *publish)
11141186
{
1187+
int rc;
11151188
word16 total_len;
11161189
byte *rx_payload = rx_buf;
11171190
byte flags = 0, type;
@@ -1125,7 +1198,12 @@ int SN_Decode_Publish(byte *rx_buf, int rx_buf_len, SN_Publish *publish)
11251198
total_len = *rx_payload++;
11261199
if (total_len == SN_PACKET_LEN_IND) {
11271200
/* The length is stored in the next two bytes */
1128-
rx_payload += MqttDecode_Num(rx_payload, &total_len, (word32)(rx_buf_len - (rx_payload - rx_buf)));
1201+
rc = MqttDecode_Num(rx_payload, &total_len,
1202+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
1203+
if (rc < 0) {
1204+
return rc;
1205+
}
1206+
rx_payload += rc;
11291207
}
11301208

11311209
if (total_len > rx_buf_len) {
@@ -1148,7 +1226,12 @@ int SN_Decode_Publish(byte *rx_buf, int rx_buf_len, SN_Publish *publish)
11481226
rx_payload += MQTT_DATA_LEN_SIZE;
11491227
publish->topic_name_len = MQTT_DATA_LEN_SIZE;
11501228

1151-
rx_payload += MqttDecode_Num(rx_payload, &publish->packet_id, (word32)(rx_buf_len - (rx_payload - rx_buf)));
1229+
rc = MqttDecode_Num(rx_payload, &publish->packet_id,
1230+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
1231+
if (rc < 0) {
1232+
return rc;
1233+
}
1234+
rx_payload += rc;
11521235

11531236
/* Set flags */
11541237
publish->duplicate = flags & SN_PACKET_FLAG_DUPLICATE;
@@ -1214,7 +1297,7 @@ int SN_Encode_PublishResp(byte* tx_buf, int tx_buf_len, byte type,
12141297
int SN_Decode_PublishResp(byte* rx_buf, int rx_buf_len, byte type,
12151298
SN_PublishResp *publish_resp)
12161299
{
1217-
int total_len;
1300+
int rc, total_len;
12181301
byte rec_type, *rx_payload = rx_buf;
12191302

12201303
/* Validate required arguments */
@@ -1238,10 +1321,20 @@ int SN_Decode_PublishResp(byte* rx_buf, int rx_buf_len, byte type,
12381321

12391322
if (publish_resp) {
12401323
if (type == SN_MSG_TYPE_PUBACK) {
1241-
rx_payload += MqttDecode_Num(rx_payload, &publish_resp->topicId, (word32)(rx_buf_len - (rx_payload - rx_buf)));
1324+
rc = MqttDecode_Num(rx_payload, &publish_resp->topicId,
1325+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
1326+
if (rc < 0) {
1327+
return rc;
1328+
}
1329+
rx_payload += rc;
12421330
}
12431331

1244-
rx_payload += MqttDecode_Num(rx_payload, &publish_resp->packet_id, (word32)(rx_buf_len - (rx_payload - rx_buf)));
1332+
rc = MqttDecode_Num(rx_payload, &publish_resp->packet_id,
1333+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
1334+
if (rc < 0) {
1335+
return rc;
1336+
}
1337+
rx_payload += rc;
12451338

12461339
if (type == SN_MSG_TYPE_PUBACK) {
12471340
publish_resp->return_code = *rx_payload++;
@@ -1330,6 +1423,7 @@ int SN_Encode_Unsubscribe(byte *tx_buf, int tx_buf_len,
13301423
int SN_Decode_UnsubscribeAck(byte *rx_buf, int rx_buf_len,
13311424
SN_UnsubscribeAck *unsubscribe_ack)
13321425
{
1426+
int rc;
13331427
word16 total_len;
13341428
byte *rx_payload = rx_buf, type;
13351429

@@ -1355,7 +1449,12 @@ int SN_Decode_UnsubscribeAck(byte *rx_buf, int rx_buf_len,
13551449

13561450
/* Decode SubAck fields */
13571451
if (unsubscribe_ack) {
1358-
rx_payload += MqttDecode_Num(rx_payload, &unsubscribe_ack->packet_id, (word32)(rx_buf_len - (rx_payload - rx_buf)));
1452+
rc = MqttDecode_Num(rx_payload, &unsubscribe_ack->packet_id,
1453+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
1454+
if (rc < 0) {
1455+
return rc;
1456+
}
1457+
rx_payload += rc;
13591458
}
13601459
(void)rx_payload;
13611460

@@ -1367,7 +1466,7 @@ int SN_Encode_Disconnect(byte *tx_buf, int tx_buf_len,
13671466
SN_Disconnect* disconnect)
13681467
{
13691468
int total_len = 2; /* length and message type */
1370-
byte *tx_payload = tx_buf;;
1469+
byte *tx_payload = tx_buf;
13711470

13721471
/* Validate required arguments */
13731472
if (tx_buf == NULL) {
@@ -1542,7 +1641,10 @@ int SN_Packet_Read(MqttClient *client, byte* rx_buf, int rx_buf_len,
15421641
len = rc;
15431642
}
15441643

1545-
(void)MqttDecode_Num(&rx_buf[1], &total_len, (word32)(rx_buf_len - 1));
1644+
rc = MqttDecode_Num(&rx_buf[1], &total_len, (word32)(rx_buf_len - 1));
1645+
if (rc < 0) {
1646+
return MqttPacket_HandleNetError(client, rc);
1647+
}
15461648
client->packet.header_len = len;
15471649
}
15481650
else {

0 commit comments

Comments
 (0)