Skip to content

Commit a7841b6

Browse files
dgarskedgarske
authored
Merge pull request #473 from embhorn/fenrir_3.16
Fixes for various fenrir issues
2 parents 2750bcb + 8928c85 commit a7841b6

4 files changed

Lines changed: 113 additions & 46 deletions

File tree

‎src/mqtt_packet.c‎

Lines changed: 39 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2606,36 +2606,36 @@ int MqttDecode_Auth(byte *rx_buf, int rx_buf_len, MqttAuth *auth)
26062606
}
26072607
rx_payload = &rx_buf[header_len];
26082608

2609-
/* Decode variable header */
2610-
auth->reason_code = *rx_payload++;
2611-
if ((auth->reason_code == MQTT_REASON_SUCCESS) ||
2612-
(auth->reason_code == MQTT_REASON_CONT_AUTH))
2613-
{
2614-
auth->props = NULL;
2615-
2616-
/* Decode Length of Properties */
2617-
if (rx_buf_len < (rx_payload - rx_buf)) {
2618-
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
2619-
}
2620-
tmp = MqttDecode_Vbi(rx_payload, &props_len,
2621-
(word32)(rx_buf_len - (rx_payload - rx_buf)));
2622-
if (tmp < 0)
2623-
return tmp;
2624-
2625-
if (props_len <= (word32)(rx_buf_len - (rx_payload - rx_buf))) {
2626-
rx_payload += tmp;
2627-
if ((rx_payload - rx_buf) > rx_buf_len) {
2609+
auth->props = NULL;
2610+
if (remain_len > 0) {
2611+
/* Decode variable header */
2612+
auth->reason_code = *rx_payload++;
2613+
if ((auth->reason_code == MQTT_REASON_SUCCESS) ||
2614+
(auth->reason_code == MQTT_REASON_CONT_AUTH))
2615+
{
2616+
/* Decode Length of Properties */
2617+
if (rx_buf_len < (rx_payload - rx_buf)) {
26282618
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
26292619
}
2630-
if (props_len > 0) {
2631-
/* Decode the Properties */
2632-
tmp = MqttDecode_Props(MQTT_PACKET_TYPE_AUTH,
2633-
&auth->props, rx_payload,
2634-
(word32)(rx_buf_len - (rx_payload - rx_buf)),
2635-
props_len);
2636-
if (tmp < 0)
2637-
return tmp;
2620+
tmp = MqttDecode_Vbi(rx_payload, &props_len,
2621+
(word32)(rx_buf_len - (rx_payload - rx_buf)));
2622+
if (tmp < 0)
2623+
return tmp;
2624+
2625+
if (props_len <= (word32)(rx_buf_len - (rx_payload - rx_buf))) {
26382626
rx_payload += tmp;
2627+
if ((rx_payload - rx_buf) > rx_buf_len) {
2628+
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
2629+
}
2630+
if (props_len > 0) {
2631+
/* Decode the Properties */
2632+
tmp = MqttDecode_Props(MQTT_PACKET_TYPE_AUTH,
2633+
&auth->props, rx_payload,
2634+
(word32)(rx_buf_len - (rx_payload - rx_buf)),
2635+
props_len);
2636+
if (tmp < 0)
2637+
return tmp;
2638+
rx_payload += tmp;
26392639
}
26402640
else if (auth->reason_code != MQTT_REASON_SUCCESS) {
26412641
/* The Reason Code and Property Length can be omitted if the
@@ -2657,9 +2657,15 @@ int MqttDecode_Auth(byte *rx_buf, int rx_buf_len, MqttAuth *auth)
26572657
else {
26582658
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
26592659
}
2660+
}
2661+
else {
2662+
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
2663+
}
26602664
}
26612665
else {
2662-
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
2666+
/* Per MQTT 5.0 section 3.15.2: Remaining Length of 0 implies
2667+
Reason Code of 0x00 (Success) with no Properties */
2668+
auth->reason_code = MQTT_REASON_SUCCESS;
26632669
}
26642670

26652671
(void)rx_payload;
@@ -2668,6 +2674,8 @@ int MqttDecode_Auth(byte *rx_buf, int rx_buf_len, MqttAuth *auth)
26682674
return header_len + remain_len;
26692675
}
26702676

2677+
/* Must be called once from a single thread before any concurrent access
2678+
* to MQTTv5 property functions. Not thread-safe if called concurrently. */
26712679
int MqttProps_Init(void)
26722680
{
26732681
int ret = MQTT_CODE_SUCCESS;
@@ -2680,6 +2688,9 @@ int MqttProps_Init(void)
26802688
return ret;
26812689
}
26822690

2691+
/* Must be called once from a single thread after all concurrent access
2692+
* to MQTTv5 property functions has ceased. Not thread-safe if called
2693+
* concurrently. */
26832694
int MqttProps_ShutDown(void)
26842695
{
26852696
int ret = MQTT_CODE_SUCCESS;

‎src/mqtt_sn_client.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1235,6 +1235,7 @@ int SN_Client_WillTopicUpdate(MqttClient *client, SN_Will *will)
12351235
wm_SemUnlock(&client->lockClient);
12361236
}
12371237
#endif
1238+
return rc;
12381239
}
12391240
#ifdef WOLFMQTT_MULTITHREAD
12401241
wm_SemUnlock(&client->lockSend);
@@ -1321,6 +1322,7 @@ int SN_Client_WillMsgUpdate(MqttClient *client, SN_Will *will)
13211322
wm_SemUnlock(&client->lockClient);
13221323
}
13231324
#endif
1325+
return rc;
13241326
}
13251327
#ifdef WOLFMQTT_MULTITHREAD
13261328
wm_SemUnlock(&client->lockSend);
@@ -1663,6 +1665,7 @@ int SN_Client_Unsubscribe(MqttClient *client, SN_Unsubscribe *unsubscribe)
16631665
wm_SemUnlock(&client->lockClient);
16641666
}
16651667
#endif
1668+
return rc;
16661669
}
16671670
#ifdef WOLFMQTT_MULTITHREAD
16681671
wm_SemUnlock(&client->lockSend);

‎src/mqtt_sn_packet.c‎

Lines changed: 66 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -183,13 +183,16 @@ int SN_Decode_Advertise(byte *rx_buf, int rx_buf_len, SN_Advertise *gw_info)
183183

184184
/* Decode fixed header */
185185
total_len = *rx_payload++;
186-
187-
/* Check message type */
188-
type = *rx_payload++;
189186
if (total_len != 5) {
190187
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
191188
}
192189

190+
if (total_len > rx_buf_len) {
191+
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
192+
}
193+
194+
/* Check message type */
195+
type = *rx_payload++;
193196
if (type != SN_MSG_TYPE_ADVERTISE) {
194197
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_PACKET_TYPE);
195198
}
@@ -354,6 +357,10 @@ int SN_Decode_WillTopicReq(byte *rx_buf, int rx_buf_len)
354357
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
355358
}
356359

360+
if (total_len > rx_buf_len) {
361+
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
362+
}
363+
357364
type = *rx_payload++;
358365
if (type != SN_MSG_TYPE_WILLTOPICREQ) {
359366
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_PACKET_TYPE);
@@ -450,6 +457,10 @@ int SN_Decode_WillMsgReq(byte *rx_buf, int rx_buf_len)
450457
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
451458
}
452459

460+
if (total_len > rx_buf_len) {
461+
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
462+
}
463+
453464
/* Message Type */
454465
type = *rx_payload++;
455466
if (type != SN_MSG_TYPE_WILLMSGREQ) {
@@ -582,7 +593,7 @@ int SN_Decode_WillTopicResponse(byte *rx_buf, int rx_buf_len, byte *ret_code)
582593
byte *rx_payload = rx_buf, type;
583594

584595
/* Validate required arguments */
585-
if (rx_buf == NULL || rx_buf_len <= 0) {
596+
if (rx_buf == NULL || rx_buf_len <= 0 || ret_code == NULL) {
586597
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_BAD_ARG);
587598
}
588599

@@ -592,6 +603,10 @@ int SN_Decode_WillTopicResponse(byte *rx_buf, int rx_buf_len, byte *ret_code)
592603
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
593604
}
594605

606+
if (total_len > rx_buf_len) {
607+
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
608+
}
609+
595610
type = *rx_payload++;
596611
if (type != SN_MSG_TYPE_WILLTOPICRESP) {
597612
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_PACKET_TYPE);
@@ -659,7 +674,7 @@ int SN_Decode_WillMsgResponse(byte *rx_buf, int rx_buf_len, byte *ret_code)
659674
byte *rx_payload = rx_buf, type;
660675

661676
/* Validate required arguments */
662-
if (rx_buf == NULL || rx_buf_len <= 0) {
677+
if (rx_buf == NULL || rx_buf_len <= 0 || ret_code == NULL) {
663678
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_BAD_ARG);
664679
}
665680

@@ -669,6 +684,10 @@ int SN_Decode_WillMsgResponse(byte *rx_buf, int rx_buf_len, byte *ret_code)
669684
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
670685
}
671686

687+
if (total_len > rx_buf_len) {
688+
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
689+
}
690+
672691
type = *rx_payload++;
673692
if (type != SN_MSG_TYPE_WILLMSGRESP) {
674693
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_PACKET_TYPE);
@@ -698,6 +717,10 @@ int SN_Decode_ConnectAck(byte *rx_buf, int rx_buf_len,
698717
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
699718
}
700719

720+
if (total_len > rx_buf_len) {
721+
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
722+
}
723+
701724
type = *rx_payload++;
702725
if (type != SN_MSG_TYPE_CONNACK) {
703726
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_PACKET_TYPE);
@@ -875,6 +898,10 @@ int SN_Decode_RegAck(byte *rx_buf, int rx_buf_len, SN_RegAck *regack)
875898
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
876899
}
877900

901+
if (total_len > rx_buf_len) {
902+
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
903+
}
904+
878905
type = *rx_payload++;
879906
if (type != SN_MSG_TYPE_REGACK) {
880907
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_PACKET_TYPE);
@@ -987,9 +1014,13 @@ int SN_Decode_SubscribeAck(byte* rx_buf, int rx_buf_len,
9871014
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
9881015
}
9891016

1017+
if (total_len > rx_buf_len) {
1018+
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
1019+
}
1020+
9901021
type = *rx_payload++;
9911022
if (type != SN_MSG_TYPE_SUBACK) {
992-
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
1023+
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_PACKET_TYPE);
9931024
}
9941025

9951026
/* Decode SubAck fields */
@@ -1063,7 +1094,9 @@ int SN_Encode_Publish(byte *tx_buf, int tx_buf_len, SN_Publish *publish)
10631094
}
10641095
else {
10651096
/* Topic ID */
1066-
tx_payload += MqttEncode_Num(tx_payload, *(word16*)publish->topic_name);
1097+
word16 topic_id;
1098+
XMEMCPY(&topic_id, publish->topic_name, sizeof(topic_id));
1099+
tx_payload += MqttEncode_Num(tx_payload, topic_id);
10671100
}
10681101

10691102
tx_payload += MqttEncode_Num(tx_payload, publish->packet_id);
@@ -1128,9 +1161,12 @@ int SN_Decode_Publish(byte *rx_buf, int rx_buf_len, SN_Publish *publish)
11281161

11291162
publish->topic_type = flags & SN_PACKET_FLAG_TOPICIDTYPE_MASK;
11301163

1131-
/* Decode payload */
1132-
1133-
publish->total_len = total_len - 7;
1164+
/* Decode payload: use pointer difference to account for both short (7)
1165+
* and extended-length (9) header formats */
1166+
if (total_len < (word16)(rx_payload - rx_buf)) {
1167+
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
1168+
}
1169+
publish->total_len = total_len - (word16)(rx_payload - rx_buf);
11341170
publish->buffer = rx_payload;
11351171
publish->buffer_pos = 0;
11361172
publish->buffer_len = publish->total_len;
@@ -1310,6 +1346,10 @@ int SN_Decode_UnsubscribeAck(byte *rx_buf, int rx_buf_len,
13101346
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
13111347
}
13121348

1349+
if (total_len > rx_buf_len) {
1350+
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
1351+
}
1352+
13131353
type = *rx_payload++;
13141354
if (type != SN_MSG_TYPE_UNSUBACK) {
13151355
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_PACKET_TYPE);
@@ -1374,6 +1414,10 @@ int SN_Decode_Disconnect(byte *rx_buf, int rx_buf_len)
13741414
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
13751415
}
13761416

1417+
if (total_len > rx_buf_len) {
1418+
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
1419+
}
1420+
13771421
type = *rx_payload++;
13781422
if (type != SN_MSG_TYPE_DISCONNECT) {
13791423
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_PACKET_TYPE);
@@ -1434,6 +1478,10 @@ int SN_Decode_Ping(byte *rx_buf, int rx_buf_len)
14341478
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
14351479
}
14361480

1481+
if (total_len > rx_buf_len) {
1482+
return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
1483+
}
1484+
14371485
type = *rx_payload++;
14381486
if ((type != SN_MSG_TYPE_PING_REQ) &&
14391487
(type != SN_MSG_TYPE_PING_RESP)) {
@@ -1510,12 +1558,6 @@ int SN_Packet_Read(MqttClient *client, byte* rx_buf, int rx_buf_len,
15101558
case MQTT_PK_READ_HEAD:
15111559
{
15121560
client->packet.stat = MQTT_PK_READ_HEAD;
1513-
}
1514-
FALL_THROUGH;
1515-
1516-
case MQTT_PK_READ:
1517-
{
1518-
client->packet.stat = MQTT_PK_READ;
15191561

15201562
if (total_len > len) {
15211563
client->packet.remain_len = total_len - len;
@@ -1539,14 +1581,20 @@ int SN_Packet_Read(MqttClient *client, byte* rx_buf, int rx_buf_len,
15391581
client->packet.remain_len = rx_buf_len -
15401582
client->packet.header_len;
15411583
}
1584+
}
1585+
FALL_THROUGH;
1586+
1587+
case MQTT_PK_READ:
1588+
{
1589+
client->packet.stat = MQTT_PK_READ;
1590+
15421591
if (MqttClient_Flags(client,0,0) & MQTT_CLIENT_FLAG_IS_DTLS) {
1543-
total_len -= client->packet.header_len;
15441592
idx = client->packet.header_len;
15451593
}
15461594
/* Read whole message */
15471595
if (client->packet.remain_len > 0) {
15481596
rc = MqttSocket_Read(client, &rx_buf[idx],
1549-
total_len, timeout_ms);
1597+
client->packet.remain_len, timeout_ms);
15501598
if (rc <= 0) {
15511599
return MqttPacket_HandleNetError(client, rc);
15521600
}

‎wolfmqtt/mqtt_packet.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -708,7 +708,12 @@ WOLFMQTT_API int MqttEncode_Props(MqttPacketType packet, MqttProp* props,
708708
byte* buf);
709709
WOLFMQTT_API int MqttDecode_Props(MqttPacketType packet, MqttProp** props,
710710
byte* buf, word32 buf_len, word32 prop_len);
711+
/* Must be called once from a single thread before any concurrent access
712+
* to MQTTv5 property functions. Not thread-safe if called concurrently. */
711713
WOLFMQTT_API int MqttProps_Init(void);
714+
/* Must be called once from a single thread after all concurrent access
715+
* to MQTTv5 property functions has ceased. Not thread-safe if called
716+
* concurrently. */
712717
WOLFMQTT_API int MqttProps_ShutDown(void);
713718
WOLFMQTT_API MqttProp* MqttProps_Add(MqttProp **head);
714719
WOLFMQTT_API int MqttProps_Free(MqttProp *head);

0 commit comments

Comments
 (0)