From 86182fe42944a50483d36ca83a05abdb3903bdab Mon Sep 17 00:00:00 2001 From: Huong Pham Date: Mon, 11 Aug 2025 15:12:20 +0300 Subject: [PATCH 1/6] merge with master --- libcanard/canard.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/libcanard/canard.c b/libcanard/canard.c index 61992742..46382539 100644 --- a/libcanard/canard.c +++ b/libcanard/canard.c @@ -43,6 +43,8 @@ #define CAVL2_T struct CanardTreeNode #define CAVL2_ASSERT(x) CANARD_ASSERT(x) // NOSONAR #include +#include +#include // --------------------------------------------- COMMON DEFINITIONS --------------------------------------------- @@ -1384,6 +1386,13 @@ int8_t canardRxSubscribe(struct CanardInstance* const ins, { int8_t out = -CANARD_ERROR_INVALID_ARGUMENT; const size_t tk = (size_t) transfer_kind; + + assert(port_id >= 49152 && port_id <= 65535); + if (port_id < 49152 || port_id > 65535) + { + fprintf(stderr, "Invalid port: %d. Port should be within 49152 to 65535\n", port_id); + exit(EXIT_FAILURE); + } if ((ins != NULL) && (out_subscription != NULL) && (tk < CANARD_NUM_TRANSFER_KINDS)) { // Reset to the initial state. This is absolutely critical because the new payload size limit may be larger From 6cad1e94f76e4e8609bda2123ffd601b0d4f866a Mon Sep 17 00:00:00 2001 From: Huong Pham Date: Mon, 11 Aug 2025 15:33:52 +0300 Subject: [PATCH 2/6] assign port_id_minimum to a value --- libcanard/canard.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libcanard/canard.c b/libcanard/canard.c index 46382539..c6d345ae 100644 --- a/libcanard/canard.c +++ b/libcanard/canard.c @@ -81,6 +81,7 @@ typedef uint16_t TransferCRC; #define CRC_INITIAL 0xFFFFU #define CRC_RESIDUE 0x0000U #define CRC_SIZE_BYTES 2U +#define PORT_ID_MINIMUM 49152 #if (CANARD_CRC_TABLE != 0) static const uint16_t CRCTable[256] = { @@ -1387,10 +1388,9 @@ int8_t canardRxSubscribe(struct CanardInstance* const ins, int8_t out = -CANARD_ERROR_INVALID_ARGUMENT; const size_t tk = (size_t) transfer_kind; - assert(port_id >= 49152 && port_id <= 65535); - if (port_id < 49152 || port_id > 65535) + if (port_id < PORT_ID_MINIMUM) // port_id can't be over 65535 because it exceeds type unsigned short maximum value. { - fprintf(stderr, "Invalid port: %d. Port should be within 49152 to 65535\n", port_id); + (void) fprintf(stderr, "Invalid port: %d. Port should be within 49152 to 65535\n", port_id); exit(EXIT_FAILURE); } if ((ins != NULL) && (out_subscription != NULL) && (tk < CANARD_NUM_TRANSFER_KINDS)) From 4ec56858b91dcb6984d5bff3c003b0ffa73fe34d Mon Sep 17 00:00:00 2001 From: Huong Pham Date: Mon, 11 Aug 2025 22:57:08 +0300 Subject: [PATCH 3/6] fix the logic part --- libcanard/canard.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/libcanard/canard.c b/libcanard/canard.c index c6d345ae..bc47d4b0 100644 --- a/libcanard/canard.c +++ b/libcanard/canard.c @@ -1388,12 +1388,9 @@ int8_t canardRxSubscribe(struct CanardInstance* const ins, int8_t out = -CANARD_ERROR_INVALID_ARGUMENT; const size_t tk = (size_t) transfer_kind; - if (port_id < PORT_ID_MINIMUM) // port_id can't be over 65535 because it exceeds type unsigned short maximum value. - { - (void) fprintf(stderr, "Invalid port: %d. Port should be within 49152 to 65535\n", port_id); - exit(EXIT_FAILURE); - } - if ((ins != NULL) && (out_subscription != NULL) && (tk < CANARD_NUM_TRANSFER_KINDS)) + if ((ins != NULL) && (out_subscription != NULL) && (tk < CANARD_NUM_TRANSFER_KINDS) && + ((transfer_kind == CanardTransferKindMessage && port_id <= CANARD_SUBJECT_ID_MAX) || + (port_id <= CANARD_SERVICE_ID_MAX))) { // Reset to the initial state. This is absolutely critical because the new payload size limit may be larger // than the old value; if there are any payload buffers allocated, we may overrun them because they are shorter From e7dd3a5a9ebbec45a2d911898a1911f4892eacc1 Mon Sep 17 00:00:00 2001 From: Huong Pham Date: Tue, 12 Aug 2025 12:40:27 +0300 Subject: [PATCH 4/6] add port id check test --- tests/test_public_rx.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_public_rx.cpp b/tests/test_public_rx.cpp index 371306fa..40a9d688 100644 --- a/tests/test_public_rx.cpp +++ b/tests/test_public_rx.cpp @@ -369,6 +369,10 @@ TEST_CASE("RxSubscriptionErrors") REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == canardRxSubscribe(&ins.getInstance(), kind.value, 0, 0, 0, &sub)); REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == canardRxSubscribe(&ins.getInstance(), CanardTransferKindMessage, 0, 0, 0, nullptr)); + REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == + canardRxSubscribe(&ins.getInstance(), CanardTransferKindMessage, 8192, 0, 0, &sub)); + REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == + canardRxSubscribe(&ins.getInstance(), CanardTransferKindResponse, 512, 0, 0, &sub)); REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == canardRxUnsubscribe(nullptr, CanardTransferKindMessage, 0)); REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == canardRxUnsubscribe(&ins.getInstance(), kind.value, 0)); From a48f9bbf62f7568a47cafc86a6d5e0bfd170e5a8 Mon Sep 17 00:00:00 2001 From: Huong Pham Date: Tue, 12 Aug 2025 13:11:48 +0300 Subject: [PATCH 5/6] fix sonar errors --- libcanard/canard.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/libcanard/canard.c b/libcanard/canard.c index bc47d4b0..5a28edb4 100644 --- a/libcanard/canard.c +++ b/libcanard/canard.c @@ -43,8 +43,6 @@ #define CAVL2_T struct CanardTreeNode #define CAVL2_ASSERT(x) CANARD_ASSERT(x) // NOSONAR #include -#include -#include // --------------------------------------------- COMMON DEFINITIONS --------------------------------------------- @@ -1389,7 +1387,7 @@ int8_t canardRxSubscribe(struct CanardInstance* const ins, const size_t tk = (size_t) transfer_kind; if ((ins != NULL) && (out_subscription != NULL) && (tk < CANARD_NUM_TRANSFER_KINDS) && - ((transfer_kind == CanardTransferKindMessage && port_id <= CANARD_SUBJECT_ID_MAX) || + (((transfer_kind == CanardTransferKindMessage) && (port_id <= CANARD_SUBJECT_ID_MAX)) || (port_id <= CANARD_SERVICE_ID_MAX))) { // Reset to the initial state. This is absolutely critical because the new payload size limit may be larger From 3d9e647a464431aab2a33636ae49091f0e0eda5a Mon Sep 17 00:00:00 2001 From: Huong Pham Date: Tue, 12 Aug 2025 16:19:48 +0300 Subject: [PATCH 6/6] add bool port_id_ok --- libcanard/canard.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/libcanard/canard.c b/libcanard/canard.c index 5a28edb4..5a555bc5 100644 --- a/libcanard/canard.c +++ b/libcanard/canard.c @@ -79,7 +79,6 @@ typedef uint16_t TransferCRC; #define CRC_INITIAL 0xFFFFU #define CRC_RESIDUE 0x0000U #define CRC_SIZE_BYTES 2U -#define PORT_ID_MINIMUM 49152 #if (CANARD_CRC_TABLE != 0) static const uint16_t CRCTable[256] = { @@ -1383,12 +1382,11 @@ int8_t canardRxSubscribe(struct CanardInstance* const ins, const CanardMicrosecond transfer_id_timeout_usec, struct CanardRxSubscription* const out_subscription) { - int8_t out = -CANARD_ERROR_INVALID_ARGUMENT; - const size_t tk = (size_t) transfer_kind; - - if ((ins != NULL) && (out_subscription != NULL) && (tk < CANARD_NUM_TRANSFER_KINDS) && - (((transfer_kind == CanardTransferKindMessage) && (port_id <= CANARD_SUBJECT_ID_MAX)) || - (port_id <= CANARD_SERVICE_ID_MAX))) + int8_t out = -CANARD_ERROR_INVALID_ARGUMENT; + const size_t tk = (size_t) transfer_kind; + const bool port_id_ok = ((transfer_kind == CanardTransferKindMessage) && (port_id <= CANARD_SUBJECT_ID_MAX)) || + (port_id <= CANARD_SERVICE_ID_MAX); + if ((ins != NULL) && (out_subscription != NULL) && (tk < CANARD_NUM_TRANSFER_KINDS) && port_id_ok) { // Reset to the initial state. This is absolutely critical because the new payload size limit may be larger // than the old value; if there are any payload buffers allocated, we may overrun them because they are shorter