Skip to content

Commit ab2290a

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

1 file changed

Lines changed: 117 additions & 23 deletions

File tree

‎src/mqtt_sn_packet.c‎

Lines changed: 117 additions & 23 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) {
@@ -127,18 +132,30 @@ int SN_Decode_Header(byte *rx_buf, int rx_buf_len,
127132
case SN_MSG_TYPE_REGACK:
128133
case SN_MSG_TYPE_PUBACK:
129134
/* octet 4-5 */
130-
MqttDecode_Num(rx_buf + 2, p_packet_id, (word32)(rx_buf_len - 3));
135+
rc = MqttDecode_Num(rx_buf + 2, p_packet_id,
136+
(word32)(rx_buf_len - 3));
137+
if (rc < 0) {
138+
return rc;
139+
}
131140
break;
132141
case SN_MSG_TYPE_PUBCOMP:
133142
case SN_MSG_TYPE_PUBREC:
134143
case SN_MSG_TYPE_PUBREL:
135144
case SN_MSG_TYPE_UNSUBACK:
136145
/* octet 2-3 */
137-
MqttDecode_Num(rx_buf, p_packet_id, (word32)(rx_buf_len - 1));
146+
rc = MqttDecode_Num(rx_buf, p_packet_id,
147+
(word32)(rx_buf_len - 1));
148+
if (rc < 0) {
149+
return rc;
150+
}
138151
break;
139152
case SN_MSG_TYPE_SUBACK:
140153
/* octet 5-6 */
141-
MqttDecode_Num(rx_buf + 3, p_packet_id, (word32)(rx_buf_len - 4));
154+
rc = MqttDecode_Num(rx_buf + 3, p_packet_id,
155+
(word32)(rx_buf_len - 4));
156+
if (rc < 0) {
157+
return rc;
158+
}
142159
break;
143160
case SN_MSG_TYPE_ADVERTISE:
144161
case SN_MSG_TYPE_SEARCHGW:
@@ -199,9 +216,15 @@ int SN_Decode_Advertise(byte *rx_buf, int rx_buf_len, SN_Advertise *gw_info)
199216

200217
/* Decode gateway info */
201218
if (gw_info != NULL) {
219+
int rc;
202220
gw_info->gwId = *rx_payload++;
203221

204-
rx_payload += MqttDecode_Num(rx_payload, &gw_info->duration, (word32)(rx_buf_len - (rx_payload - rx_buf)));
222+
rc = MqttDecode_Num(rx_payload, &gw_info->duration,
223+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
224+
if (rc < 0) {
225+
return rc;
226+
}
227+
rx_payload += rc;
205228
}
206229
(void)rx_payload;
207230

@@ -238,7 +261,7 @@ int SN_Encode_SearchGW(byte *tx_buf, int tx_buf_len, byte hops)
238261

239262
int SN_Decode_GWInfo(byte *rx_buf, int rx_buf_len, SN_GwInfo *gw_info)
240263
{
241-
int total_len;
264+
int rc, total_len;
242265
byte *rx_payload = rx_buf, type;
243266

244267
/* Validate required arguments */
@@ -250,7 +273,12 @@ int SN_Decode_GWInfo(byte *rx_buf, int rx_buf_len, SN_GwInfo *gw_info)
250273
total_len = *rx_payload++;
251274
if (total_len == SN_PACKET_LEN_IND) {
252275
/* The length is stored in the next two bytes */
253-
rx_payload += MqttDecode_Num(rx_payload, (word16*)&total_len, (word32)(rx_buf_len - (rx_payload - rx_buf)));
276+
rc = MqttDecode_Num(rx_payload, (word16*)&total_len,
277+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
278+
if (rc < 0) {
279+
return rc;
280+
}
281+
rx_payload += rc;
254282
}
255283

256284
if (total_len > rx_buf_len) {
@@ -794,7 +822,7 @@ int SN_Encode_Register(byte *tx_buf, int tx_buf_len, SN_Register *regist)
794822

795823
int SN_Decode_Register(byte *rx_buf, int rx_buf_len, SN_Register *regist)
796824
{
797-
int total_len;
825+
int rc, total_len;
798826
byte *rx_payload = rx_buf, type;
799827

800828
/* Validate required arguments */
@@ -806,7 +834,12 @@ int SN_Decode_Register(byte *rx_buf, int rx_buf_len, SN_Register *regist)
806834
total_len = *rx_payload++;
807835
if (total_len == SN_PACKET_LEN_IND) {
808836
/* The length is stored in the next two bytes */
809-
rx_payload += MqttDecode_Num(rx_payload, (word16*)&total_len, (word32)(rx_buf_len - (rx_payload - rx_buf)));
837+
rc = MqttDecode_Num(rx_payload, (word16*)&total_len,
838+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
839+
if (rc < 0) {
840+
return rc;
841+
}
842+
rx_payload += rc;
810843
}
811844

812845
if (total_len >= rx_buf_len) {
@@ -824,10 +857,20 @@ int SN_Decode_Register(byte *rx_buf, int rx_buf_len, SN_Register *regist)
824857

825858
if (regist != NULL) {
826859
/* Decode Topic ID assigned by GW */
827-
rx_payload += MqttDecode_Num(rx_payload, &regist->topicId, (word32)(rx_buf_len - (rx_payload - rx_buf)));
860+
rc = MqttDecode_Num(rx_payload, &regist->topicId,
861+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
862+
if (rc < 0) {
863+
return rc;
864+
}
865+
rx_payload += rc;
828866

829867
/* Decode packet ID */
830-
rx_payload += MqttDecode_Num(rx_payload, &regist->packet_id, (word32)(rx_buf_len - (rx_payload - rx_buf)));
868+
rc = MqttDecode_Num(rx_payload, &regist->packet_id,
869+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
870+
if (rc < 0) {
871+
return rc;
872+
}
873+
rx_payload += rc;
831874

832875
/* Decode Topic Name */
833876
regist->topicName = (char*)rx_payload;
@@ -884,7 +927,7 @@ int SN_Encode_RegAck(byte *tx_buf, int tx_buf_len, SN_RegAck *regack)
884927

885928
int SN_Decode_RegAck(byte *rx_buf, int rx_buf_len, SN_RegAck *regack)
886929
{
887-
int total_len;
930+
int rc, total_len;
888931
byte *rx_payload = rx_buf, type;
889932

890933
/* Validate required arguments */
@@ -909,10 +952,20 @@ int SN_Decode_RegAck(byte *rx_buf, int rx_buf_len, SN_RegAck *regack)
909952

910953
if (regack != NULL) {
911954
/* Decode Topic ID assigned by GW */
912-
rx_payload += MqttDecode_Num(rx_payload, &regack->topicId, (word32)(rx_buf_len - (rx_payload - rx_buf)));
955+
rc = MqttDecode_Num(rx_payload, &regack->topicId,
956+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
957+
if (rc < 0) {
958+
return rc;
959+
}
960+
rx_payload += rc;
913961

914962
/* Decode packet ID */
915-
rx_payload += MqttDecode_Num(rx_payload, &regack->packet_id, (word32)(rx_buf_len - (rx_payload - rx_buf)));
963+
rc = MqttDecode_Num(rx_payload, &regack->packet_id,
964+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
965+
if (rc < 0) {
966+
return rc;
967+
}
968+
rx_payload += rc;
916969

917970
/* Decode return code */
918971
regack->return_code = *rx_payload++;
@@ -1000,6 +1053,7 @@ int SN_Encode_Subscribe(byte *tx_buf, int tx_buf_len, SN_Subscribe *subscribe)
10001053
int SN_Decode_SubscribeAck(byte* rx_buf, int rx_buf_len,
10011054
SN_SubAck *subscribe_ack)
10021055
{
1056+
int rc;
10031057
word16 total_len;
10041058
byte* rx_payload = rx_buf, type;
10051059

@@ -1026,8 +1080,18 @@ int SN_Decode_SubscribeAck(byte* rx_buf, int rx_buf_len,
10261080
/* Decode SubAck fields */
10271081
if (subscribe_ack) {
10281082
subscribe_ack->flags = *rx_payload++;
1029-
rx_payload += MqttDecode_Num(rx_payload, &subscribe_ack->topicId, (word32)(rx_buf_len - (rx_payload - rx_buf)));
1030-
rx_payload += MqttDecode_Num(rx_payload, &subscribe_ack->packet_id, (word32)(rx_buf_len - (rx_payload - rx_buf)));
1083+
rc = MqttDecode_Num(rx_payload, &subscribe_ack->topicId,
1084+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
1085+
if (rc < 0) {
1086+
return rc;
1087+
}
1088+
rx_payload += rc;
1089+
rc = MqttDecode_Num(rx_payload, &subscribe_ack->packet_id,
1090+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
1091+
if (rc < 0) {
1092+
return rc;
1093+
}
1094+
rx_payload += rc;
10311095
subscribe_ack->return_code = *rx_payload++;
10321096
}
10331097
(void)rx_payload;
@@ -1119,6 +1183,7 @@ int SN_Encode_Publish(byte *tx_buf, int tx_buf_len, SN_Publish *publish)
11191183

11201184
int SN_Decode_Publish(byte *rx_buf, int rx_buf_len, SN_Publish *publish)
11211185
{
1186+
int rc;
11221187
word16 total_len;
11231188
byte *rx_payload = rx_buf;
11241189
byte flags = 0, type;
@@ -1132,7 +1197,12 @@ int SN_Decode_Publish(byte *rx_buf, int rx_buf_len, SN_Publish *publish)
11321197
total_len = *rx_payload++;
11331198
if (total_len == SN_PACKET_LEN_IND) {
11341199
/* The length is stored in the next two bytes */
1135-
rx_payload += MqttDecode_Num(rx_payload, &total_len, (word32)(rx_buf_len - (rx_payload - rx_buf)));
1200+
rc = MqttDecode_Num(rx_payload, &total_len,
1201+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
1202+
if (rc < 0) {
1203+
return rc;
1204+
}
1205+
rx_payload += rc;
11361206
}
11371207

11381208
if (total_len > rx_buf_len) {
@@ -1155,7 +1225,12 @@ int SN_Decode_Publish(byte *rx_buf, int rx_buf_len, SN_Publish *publish)
11551225
rx_payload += MQTT_DATA_LEN_SIZE;
11561226
publish->topic_name_len = MQTT_DATA_LEN_SIZE;
11571227

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

11601235
/* Set flags */
11611236
publish->duplicate = flags & SN_PACKET_FLAG_DUPLICATE;
@@ -1221,7 +1296,7 @@ int SN_Encode_PublishResp(byte* tx_buf, int tx_buf_len, byte type,
12211296
int SN_Decode_PublishResp(byte* rx_buf, int rx_buf_len, byte type,
12221297
SN_PublishResp *publish_resp)
12231298
{
1224-
int total_len;
1299+
int rc, total_len;
12251300
byte rec_type, *rx_payload = rx_buf;
12261301

12271302
/* Validate required arguments */
@@ -1245,10 +1320,20 @@ int SN_Decode_PublishResp(byte* rx_buf, int rx_buf_len, byte type,
12451320

12461321
if (publish_resp) {
12471322
if (type == SN_MSG_TYPE_PUBACK) {
1248-
rx_payload += MqttDecode_Num(rx_payload, &publish_resp->topicId, (word32)(rx_buf_len - (rx_payload - rx_buf)));
1323+
rc = MqttDecode_Num(rx_payload, &publish_resp->topicId,
1324+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
1325+
if (rc < 0) {
1326+
return rc;
1327+
}
1328+
rx_payload += rc;
12491329
}
12501330

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

12531338
if (type == SN_MSG_TYPE_PUBACK) {
12541339
publish_resp->return_code = *rx_payload++;
@@ -1338,6 +1423,7 @@ int SN_Encode_Unsubscribe(byte *tx_buf, int tx_buf_len,
13381423
int SN_Decode_UnsubscribeAck(byte *rx_buf, int rx_buf_len,
13391424
SN_UnsubscribeAck *unsubscribe_ack)
13401425
{
1426+
int rc;
13411427
word16 total_len;
13421428
byte *rx_payload = rx_buf, type;
13431429

@@ -1363,7 +1449,12 @@ int SN_Decode_UnsubscribeAck(byte *rx_buf, int rx_buf_len,
13631449

13641450
/* Decode SubAck fields */
13651451
if (unsubscribe_ack) {
1366-
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;
13671458
}
13681459
(void)rx_payload;
13691460

@@ -1550,7 +1641,10 @@ int SN_Packet_Read(MqttClient *client, byte* rx_buf, int rx_buf_len,
15501641
len = rc;
15511642
}
15521643

1553-
(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+
}
15541648
client->packet.header_len = len;
15551649
}
15561650
else {

0 commit comments

Comments
 (0)