From 906e574564917ef422ea2828e8793884e7c1bb6e Mon Sep 17 00:00:00 2001 From: Eric Blankenhorn Date: Fri, 1 Dec 2023 15:49:22 -0600 Subject: [PATCH] Fix high coverity issues --- examples/firmware/fwpush.c | 8 ++- examples/mqttsimple/mqttsimple.c | 5 +- src/mqtt_sn_client.c | 114 +++++++++++++++++++++++++++++-- src/mqtt_sn_packet.c | 14 +--- 4 files changed, 118 insertions(+), 23 deletions(-) diff --git a/examples/firmware/fwpush.c b/examples/firmware/fwpush.c index fbefbdc90..bf9fbe4b1 100644 --- a/examples/firmware/fwpush.c +++ b/examples/firmware/fwpush.c @@ -245,7 +245,13 @@ static int fw_message_build(MQTTCtx *mqttCtx, const char* fwFile, if (rc == 0) { /* Return values */ - if (p_msgBuf) *p_msgBuf = msgBuf; + if (p_msgBuf) { + *p_msgBuf = msgBuf; + } + else { + if (msgBuf) WOLFMQTT_FREE(msgBuf); + } + if (p_msgLen) *p_msgLen = msgLen; } else { diff --git a/examples/mqttsimple/mqttsimple.c b/examples/mqttsimple/mqttsimple.c index 814c8f370..5b2089ba0 100644 --- a/examples/mqttsimple/mqttsimple.c +++ b/examples/mqttsimple/mqttsimple.c @@ -169,16 +169,15 @@ static int mqtt_net_connect(void *context, const char* host, word16 port, /* prefer ip4 addresses */ while (res) { if (res->ai_family == AF_INET) { - result = res; break; } res = res->ai_next; } - if (result->ai_family == AF_INET) { + if (res) { addr.sin_port = htons(port); addr.sin_family = AF_INET; addr.sin_addr = - ((struct sockaddr_in*)(result->ai_addr))->sin_addr; + ((struct sockaddr_in*)(res->ai_addr))->sin_addr; } else { rc = -1; diff --git a/src/mqtt_sn_client.c b/src/mqtt_sn_client.c index 1144d6362..3bce70abe 100644 --- a/src/mqtt_sn_client.c +++ b/src/mqtt_sn_client.c @@ -454,6 +454,110 @@ static int SN_Client_HandlePacket(MqttClient* client, SN_MsgType packet_type, return rc; } +/* Helper for clearing the contents of an object buffer based on packet type */ +static void MqttSNClient_PacketReset(SN_MsgType packet_type, void* packet_obj) +{ + size_t objSz = 0; + size_t offset = sizeof(MqttMsgStat); + switch (packet_type) { + case SN_MSG_TYPE_ADVERTISE: + objSz = sizeof(SN_Advertise); + break; + case SN_MSG_TYPE_SEARCHGW: + objSz = sizeof(SN_SearchGw); + #ifdef WOLFMQTT_MULTITHREAD + offset += sizeof(MqttPendResp); + #endif + break; + case SN_MSG_TYPE_GWINFO: + objSz = sizeof(SN_GwInfo); + break; + case SN_MSG_TYPE_CONNECT: + objSz = sizeof(SN_Connect); + #ifdef WOLFMQTT_MULTITHREAD + offset += sizeof(MqttPendResp); + #endif + break; + case SN_MSG_TYPE_CONNACK: + objSz = sizeof(SN_ConnectAck); + break; + case SN_MSG_TYPE_WILLTOPICREQ: + case SN_MSG_TYPE_WILLTOPIC: + case SN_MSG_TYPE_WILLMSGREQ: + case SN_MSG_TYPE_WILLMSG: + objSz = sizeof(SN_Will); + #ifdef WOLFMQTT_MULTITHREAD + offset += sizeof(MqttPendResp); + #endif + break; + case SN_MSG_TYPE_REGISTER: + objSz = sizeof(SN_Register); + #ifdef WOLFMQTT_MULTITHREAD + offset += sizeof(MqttPendResp); + #endif + break; + case SN_MSG_TYPE_REGACK: + objSz = sizeof(SN_RegAck); + break; + case SN_MSG_TYPE_PUBLISH: + objSz = sizeof(SN_Publish); + #ifdef WOLFMQTT_MULTITHREAD + offset += sizeof(MqttPendResp); + #endif + break; + case SN_MSG_TYPE_PUBACK: + case SN_MSG_TYPE_PUBCOMP: + case SN_MSG_TYPE_PUBREC: + case SN_MSG_TYPE_PUBREL: + objSz = sizeof(SN_PublishResp); + break; + case SN_MSG_TYPE_SUBSCRIBE: + objSz = sizeof(SN_Subscribe); + #ifdef WOLFMQTT_MULTITHREAD + offset += sizeof(MqttPendResp); + #endif + break; + case SN_MSG_TYPE_SUBACK: + objSz = sizeof(SN_SubAck); + break; + case SN_MSG_TYPE_UNSUBSCRIBE: + objSz = sizeof(SN_Unsubscribe); + #ifdef WOLFMQTT_MULTITHREAD + offset += sizeof(MqttPendResp); + #endif + break; + case SN_MSG_TYPE_UNSUBACK: + objSz = sizeof(SN_UnsubscribeAck); + break; + case SN_MSG_TYPE_PING_REQ: + case SN_MSG_TYPE_PING_RESP: + objSz = sizeof(SN_PingReq); + #ifdef WOLFMQTT_MULTITHREAD + offset += sizeof(MqttPendResp); + #endif + break; + case SN_MSG_TYPE_DISCONNECT: + objSz = sizeof(SN_Disconnect); + break; + case SN_MSG_TYPE_WILLTOPICUPD: + case SN_MSG_TYPE_WILLTOPICRESP: + case SN_MSG_TYPE_WILLMSGUPD: + case SN_MSG_TYPE_WILLMSGRESP: + objSz = sizeof(SN_Will); + #ifdef WOLFMQTT_MULTITHREAD + offset += sizeof(MqttPendResp); + #endif + break; + case SN_MSG_TYPE_ENCAPMSG: + case SN_MSG_TYPE_ANY: + default: + break; + } /* switch (packet_type) */ + if (objSz > offset) { + XMEMSET((byte*)packet_obj + offset, 0, objSz - offset); + } +} + static int SN_Client_WaitType(MqttClient *client, void* packet_obj, byte wait_type, word16 wait_packet_id, int timeout_ms) { @@ -562,6 +666,9 @@ static int SN_Client_WaitType(MqttClient *client, void* packet_obj, break; } + /* Clear shared union for next call */ + MqttSNClient_PacketReset(packet_type, &client->msg); + #ifdef WOLFMQTT_DEBUG_CLIENT PRINTF("Read Packet: Len %d, Type %d, ID %d", client->packet.buf_len, packet_type, packet_id); @@ -683,13 +790,6 @@ static int SN_Client_WaitType(MqttClient *client, void* packet_obj, } #endif - /* Clear shared union for next call */ - if ((MqttObject*)use_packet_obj == &client->msg) { - /* reset the members, but not the stat */ - XMEMSET(((byte*)&client->msg.stat) + sizeof(client->msg.stat), 0, - sizeof(client->msg)-sizeof(client->msg.stat)); - } - if (rc < 0) { #ifdef WOLFMQTT_DEBUG_CLIENT if (rc != MQTT_CODE_CONTINUE) { diff --git a/src/mqtt_sn_packet.c b/src/mqtt_sn_packet.c index 287142a2b..bdd4bd2d2 100644 --- a/src/mqtt_sn_packet.c +++ b/src/mqtt_sn_packet.c @@ -300,22 +300,12 @@ int SN_Encode_Connect(byte *tx_buf, int tx_buf_len, SN_Connect *mc_connect) total_len += id_len; - if (total_len > SN_PACKET_MAX_SMALL_SIZE) { - total_len += 2; /* Store len in three bytes */ - } - if (total_len > tx_buf_len) { return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); } - /* Encode length */ - if (total_len <= SN_PACKET_MAX_SMALL_SIZE) { - *tx_payload++ = (byte)total_len; - } - else { - *tx_payload++ = SN_PACKET_LEN_IND; - tx_payload += MqttEncode_Num(tx_payload, total_len); - } + /* Encode length (max size is 29 bytes, so no need for var len check) */ + *tx_payload++ = (byte)total_len; /* Encode message type */ *tx_payload++ = SN_MSG_TYPE_CONNECT;