From: Abhinay Ramesh Date: Sat, 12 Feb 2022 12:05:57 +0000 (+0000) Subject: ospf6d: fix coverity issues. X-Git-Tag: pim6-testing-20220430~324^2 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=42bfee18c2484fac611501d6fe047374250ffafc;p=matthieu%2Ffrr.git ospf6d: fix coverity issues. Fixed below coverity issues ________________________________________________________________________________________________________ *** CID 1511366: (TAINTED_SCALAR) /ospf6d/ospf6_message.c: 2631 in ospf6_make_lsupdate_list() 2625 + OSPF6_HEADER_SIZE) 2626 > ospf6_packet_max(on->ospf6_if)) { 2627 ospf6_fill_header(on->ospf6_if, (*op)->s, 2628 length + OSPF6_HEADER_SIZE); 2629 (*op)->length = length + OSPF6_HEADER_SIZE; 2630 ospf6_fill_lsupdate_header((*op)->s, *lsa_cnt); >>> CID 1511366: (TAINTED_SCALAR) >>> Passing tainted variable "(*op)->length" to a tainted sink. 2631 ospf6_send_lsupdate(on, NULL, *op); 2632 2633 /* refresh packet */ 2634 *op = ospf6_packet_new(on->ospf6_if->ifmtu); 2635 length = OSPF6_LS_UPD_MIN_SIZE; 2636 *lsa_cnt = 0; /ospf6d/ospf6_message.c: 2631 in ospf6_make_lsupdate_list() 2625 + OSPF6_HEADER_SIZE) 2626 > ospf6_packet_max(on->ospf6_if)) { 2627 ospf6_fill_header(on->ospf6_if, (*op)->s, 2628 length + OSPF6_HEADER_SIZE); 2629 (*op)->length = length + OSPF6_HEADER_SIZE; 2630 ospf6_fill_lsupdate_header((*op)->s, *lsa_cnt); >>> CID 1511366: (TAINTED_SCALAR) >>> Passing tainted variable "(*op)->length" to a tainted sink. 2631 ospf6_send_lsupdate(on, NULL, *op); ________________________________________________________________________________________________________ *** CID 1511365: (TAINTED_SCALAR) /ospf6d/ospf6_message.c: 2674 in ospf6_make_ls_retrans_list() 2669 if (on->ospf6_if->state == OSPF6_INTERFACE_POINTTOPOINT) 2670 (*op)->dst = allspfrouters6; 2671 else 2672 (*op)->dst = on->linklocal_addr; 2673 >>> CID 1511365: (TAINTED_SCALAR) >>> Passing tainted variable "(*op)->length" to a tainted sink. 2674 ospf6_fill_hdr_checksum(on->ospf6_if, *op); 2675 ospf6_packet_add(on->ospf6_if, *op); 2676 OSPF6_MESSAGE_WRITE_ON(on->ospf6_if); /ospf6d/ospf6_message.c: 2674 in ospf6_make_ls_retrans_list() 2669 if (on->ospf6_if->state == OSPF6_INTERFACE_POINTTOPOINT) 2670 (*op)->dst = allspfrouters6; 2671 else 2672 (*op)->dst = on->linklocal_addr; 2673 >>> CID 1511365: (TAINTED_SCALAR) >>> Passing tainted variable "(*op)->length" to a tainted sink. 2674 ospf6_fill_hdr_checksum(on->ospf6_if, *op); 2675 ospf6_packet_add(on->ospf6_if, *op); 2676 OSPF6_MESSAGE_WRITE_ON(on->ospf6_if); /ospf6d/ospf6_message.c: 2674 in ospf6_make_ls_retrans_list() 2668 ospf6_fill_lsupdate_header((*op)->s, *lsa_cnt); 2669 if (on->ospf6_if->state == OSPF6_INTERFACE_POINTTOPOINT) 2670 (*op)->dst = allspfrouters6; 2671 else 2672 (*op)->dst = on->linklocal_addr; 2673 >>> CID 1511365: (TAINTED_SCALAR) >>> Passing tainted variable "(*op)->length" to a tainted sink. 2674 ospf6_fill_hdr_checksum(on->ospf6_if, *op); 2675 ospf6_packet_add(on->ospf6_if, *op); 2676 OSPF6_MESSAGE_WRITE_ON(on->ospf6_if); ________________________________________________________________________________________________________ *** CID 1511364: Insecure data handling (TAINTED_SCALAR) /ospf6d/ospf6_message.c: 2125 in ospf6_write() 2120 if (oi->at_data.flags != 0) { 2121 at_len = ospf6_auth_len_get(oi); 2122 if (at_len) { 2123 iovector[0].iov_len = 2124 ntohs(oh->length) + at_len; >>> CID 1511364: Insecure data handling (TAINTED_SCALAR) >>> Passing tainted variable "iovector[0].iov_len" to a tainted sink. 2125 ospf6_auth_digest_send(oi->linklocal_addr, oi, 2126 oh, at_len, 2127 iovector[0].iov_len); 2128 } else { 2129 iovector[0].iov_len = ntohs(oh->length); 2130 } ________________________________________________________________________________________________________ *** CID 1511363: (DEADCODE) /ospf6d/ospf6_auth_trailer.c: 275 in ospf6_hash_hmac_sha_digest() 269 case KEYCHAIN_ALGO_HMAC_SHA512: 270 #ifdef CRYPTO_OPENSSL 271 sha512_digest(mes, len, digest); 272 #endif 273 break; 274 case KEYCHAIN_ALGO_NULL: >>> CID 1511363: (DEADCODE) >>> Execution cannot reach this statement: "case KEYCHAIN_ALGO_MAX:". 275 case KEYCHAIN_ALGO_MAX: 276 default: /ospf6d/ospf6_auth_trailer.c: 274 in ospf6_hash_hmac_sha_digest() 269 case KEYCHAIN_ALGO_HMAC_SHA512: 270 #ifdef CRYPTO_OPENSSL 271 sha512_digest(mes, len, digest); 272 #endif 273 break; >>> CID 1511363: (DEADCODE) >>> Execution cannot reach this statement: "case KEYCHAIN_ALGO_NULL:". 274 case KEYCHAIN_ALGO_NULL: 275 case KEYCHAIN_ALGO_MAX: 276 default: ________________________________________________________________________________________________________ *** CID 1511362: Insecure data handling (TAINTED_SCALAR) /ospf6d/ospf6_auth_trailer.c: 541 in ospf6_auth_check_digest() 535 536 auth_len = ntohs(ospf6_auth->length); 537 538 memcpy(temp_hash, ospf6_auth->data, hash_len); 539 memcpy(ospf6_auth->data, apad, hash_len); 540 >>> CID 1511362: Insecure data handling (TAINTED_SCALAR) >>> Passing tainted variable "oh_len + auth_len + lls_block_len" to a tainted sink. 541 ospf6_auth_update_digest(oi, oh, ospf6_auth, auth_str, 542 (oh_len + auth_len + lls_block_len), 543 hash_algo); ________________________________________________________________________________________________________ *** CID 1511361: Insecure data handling (TAINTED_SCALAR) /ospf6d/ospf6_auth_trailer.c: 124 in ospf6_auth_hdr_dump_recv() 118 at_len = length - (oh_len + lls_len); 119 if (at_len > 0) { 120 ospf6_at_hdr = 121 (struct ospf6_auth_hdr *)((uint8_t *)ospfh + oh_len); 122 at_hdr_len = ntohs(ospf6_at_hdr->length); 123 hash_len = at_hdr_len - OSPF6_AUTH_HDR_MIN_SIZE; >>> CID 1511361: Insecure data handling (TAINTED_SCALAR) >>> Passing tainted variable "hash_len" to a tainted sink. 124 memcpy(temp, ospf6_at_hdr->data, hash_len); 125 temp[hash_len] = '\0'; ________________________________________________________________________________________________________ *** CID 1482146: Insecure data handling (TAINTED_SCALAR) /ospf6d/ospf6_message.c: 2787 in ospf6_lsupdate_send_neighbor_now() 2781 2782 if (IS_OSPF6_DEBUG_FLOODING 2783 || IS_OSPF6_DEBUG_MESSAGE(OSPF6_MESSAGE_TYPE_LSUPDATE, SEND_HDR)) 2784 zlog_debug("%s: Send lsupdate with lsa %s (age %u)", __func__, 2785 lsa->name, ntohs(lsa->header->age)); 2786 >>> CID 1482146: Insecure data handling (TAINTED_SCALAR) >>> Passing tainted variable "op->length" to a tainted sink. 2787 ospf6_send_lsupdate(on, NULL, op); Signed-off-by: Abhinay Ramesh --- diff --git a/lib/keychain.c b/lib/keychain.c index 6b03fbd560..c29c45a114 100644 --- a/lib/keychain.c +++ b/lib/keychain.c @@ -357,12 +357,12 @@ const struct keychain_algo_info algo_info[] = { KEYCHAIN_ALGO_MAX_INTERNAL_BLK_SIZE, "Not defined"} }; -uint32_t keychain_get_block_size(enum keychain_hash_algo key) +uint16_t keychain_get_block_size(enum keychain_hash_algo key) { return algo_info[key].block; } -uint32_t keychain_get_hash_len(enum keychain_hash_algo key) +uint16_t keychain_get_hash_len(enum keychain_hash_algo key) { return algo_info[key].length; } diff --git a/lib/keychain.h b/lib/keychain.h index c47bb7a792..71319d9722 100644 --- a/lib/keychain.h +++ b/lib/keychain.h @@ -54,14 +54,14 @@ enum keychain_hash_algo { struct keychain_algo_info { enum keychain_hash_algo key; const char *name; - uint32_t length; - uint32_t block; + uint16_t length; + uint16_t block; const char *desc; }; extern const struct keychain_algo_info algo_info[]; -uint32_t keychain_get_block_size(enum keychain_hash_algo key); -uint32_t keychain_get_hash_len(enum keychain_hash_algo key); +uint16_t keychain_get_block_size(enum keychain_hash_algo key); +uint16_t keychain_get_hash_len(enum keychain_hash_algo key); const char *keychain_get_description(enum keychain_hash_algo key); struct keychain_algo_info keychain_get_hash_algo_info(enum keychain_hash_algo key); diff --git a/ospf6d/ospf6_auth_trailer.c b/ospf6d/ospf6_auth_trailer.c index 2d0621cc2c..3582ef53b2 100644 --- a/ospf6d/ospf6_auth_trailer.c +++ b/ospf6d/ospf6_auth_trailer.c @@ -82,8 +82,8 @@ const uint8_t ospf6_hash_opad_max[KEYCHAIN_ALGO_MAX_INTERNAL_BLK_SIZE] = { void ospf6_auth_hdr_dump_send(struct ospf6_header *ospfh, uint16_t length) { struct ospf6_auth_hdr *ospf6_at_hdr; - int at_len, oh_len, at_hdr_len, hash_len; - unsigned char temp[KEYCHAIN_MAX_HASH_SIZE+1]; + uint16_t at_len, oh_len, at_hdr_len, hash_len; + unsigned char temp[KEYCHAIN_MAX_HASH_SIZE + 1]; oh_len = htons(ospfh->length); at_len = length - oh_len; @@ -111,7 +111,7 @@ void ospf6_auth_hdr_dump_recv(struct ospf6_header *ospfh, uint16_t length, unsigned int lls_len) { struct ospf6_auth_hdr *ospf6_at_hdr; - int at_len, oh_len, at_hdr_len, hash_len; + uint16_t at_len, oh_len, at_hdr_len, hash_len; unsigned char temp[KEYCHAIN_MAX_HASH_SIZE + 1]; oh_len = ntohs(ospfh->length); @@ -246,7 +246,7 @@ static void ospf6_hash_hmac_sha_digest(enum keychain_hash_algo key, unsigned char *mes, uint32_t len, unsigned char *digest) { - if ((key <= KEYCHAIN_ALGO_NULL) || (key >= KEYCHAIN_ALGO_MAX)) + if ((key < KEYCHAIN_ALGO_NULL) || (key > KEYCHAIN_ALGO_MAX)) return; switch (key) { @@ -279,9 +279,9 @@ static void ospf6_hash_hmac_sha_digest(enum keychain_hash_algo key, } } -unsigned int ospf6_auth_len_get(struct ospf6_interface *oi) +uint16_t ospf6_auth_len_get(struct ospf6_interface *oi) { - unsigned int at_len = 0; + uint16_t at_len = 0; char *keychain_name = NULL; struct keychain *keychain = NULL; struct key *key = NULL; @@ -481,8 +481,8 @@ int ospf6_auth_check_digest(struct ospf6_header *oh, struct ospf6_interface *oi, if (oi->at_data.flags == 0) return OSPF6_AUTH_PROCESS_NORMAL; - ospf6_auth = (struct ospf6_auth_hdr *)((uint8_t *)oh + oh_len - + lls_block_len); + ospf6_auth = (struct ospf6_auth_hdr *)((uint8_t *)oh + + (oh_len + lls_block_len)); if (CHECK_FLAG(oi->at_data.flags, OSPF6_AUTH_TRAILER_KEYCHAIN)) { keychain = keychain_lookup(oi->at_data.keychain); if (!keychain) { diff --git a/ospf6d/ospf6_auth_trailer.h b/ospf6d/ospf6_auth_trailer.h index 86ec49beba..dea4a6e168 100644 --- a/ospf6d/ospf6_auth_trailer.h +++ b/ospf6d/ospf6_auth_trailer.h @@ -70,7 +70,7 @@ void ospf6_auth_hdr_dump_recv(struct ospf6_header *ospfh, uint16_t length, unsigned int lls_len); unsigned char *ospf6_hash_message_xor(unsigned char *mes1, unsigned char *mes2, uint32_t len); -unsigned int ospf6_auth_len_get(struct ospf6_interface *oi); +uint16_t ospf6_auth_len_get(struct ospf6_interface *oi); int ospf6_auth_validate_pkt(struct ospf6_interface *oi, unsigned int *pkt_len, struct ospf6_header *oh, unsigned int *at_len, unsigned int *lls_block_len); diff --git a/ospf6d/ospf6_message.c b/ospf6d/ospf6_message.c index c1004a7080..7ebfdd26e1 100644 --- a/ospf6d/ospf6_message.c +++ b/ospf6d/ospf6_message.c @@ -2021,9 +2021,9 @@ static void ospf6_auth_trailer_copy_keychain_key(struct ospf6_interface *oi) } } -static uint32_t ospf6_packet_max(struct ospf6_interface *oi) +static uint16_t ospf6_packet_max(struct ospf6_interface *oi) { - int at_len = 0; + uint16_t at_len = 0; assert(oi->ifmtu > sizeof(struct ip6_hdr)); @@ -2092,7 +2092,7 @@ static int ospf6_write(struct thread *thread) int len; int64_t latency = 0; struct timeval timestamp; - unsigned int at_len = 0; + uint16_t at_len = 0; if (ospf6->fd < 0) { zlog_warn("ospf6_write failed to send, fd %d", ospf6->fd); @@ -2621,9 +2621,8 @@ static uint16_t ospf6_make_lsupdate_list(struct ospf6_neighbor *on, stream_forward_endp((*op)->s, OSPF6_LS_UPD_MIN_SIZE); for (ALL_LSDB(on->lsupdate_list, lsa, lsanext)) { - if ((length + (unsigned int)OSPF6_LSA_SIZE(lsa->header) - + OSPF6_HEADER_SIZE) - > ospf6_packet_max(on->ospf6_if)) { + if ((length + OSPF6_LSA_SIZE(lsa->header) + OSPF6_HEADER_SIZE) > + ospf6_packet_max(on->ospf6_if)) { ospf6_fill_header(on->ospf6_if, (*op)->s, length + OSPF6_HEADER_SIZE); (*op)->length = length + OSPF6_HEADER_SIZE; @@ -2659,9 +2658,8 @@ static uint16_t ospf6_make_ls_retrans_list(struct ospf6_neighbor *on, stream_forward_endp((*op)->s, OSPF6_LS_UPD_MIN_SIZE); for (ALL_LSDB(on->retrans_list, lsa, lsanext)) { - if ((length + (unsigned int)OSPF6_LSA_SIZE(lsa->header) - + OSPF6_HEADER_SIZE) - > ospf6_packet_max(on->ospf6_if)) { + if ((length + OSPF6_LSA_SIZE(lsa->header) + OSPF6_HEADER_SIZE) > + ospf6_packet_max(on->ospf6_if)) { ospf6_fill_header(on->ospf6_if, (*op)->s, length + OSPF6_HEADER_SIZE); (*op)->length = length + OSPF6_HEADER_SIZE; @@ -2800,9 +2798,8 @@ static uint16_t ospf6_make_lsupdate_interface(struct ospf6_interface *oi, stream_forward_endp((*op)->s, OSPF6_LS_UPD_MIN_SIZE); for (ALL_LSDB(oi->lsupdate_list, lsa, lsanext)) { - if (length + (unsigned int)OSPF6_LSA_SIZE(lsa->header) - + OSPF6_HEADER_SIZE - > ospf6_packet_max(oi)) { + if (length + OSPF6_LSA_SIZE(lsa->header) + OSPF6_HEADER_SIZE > + ospf6_packet_max(oi)) { ospf6_fill_header(oi, (*op)->s, length + OSPF6_HEADER_SIZE); (*op)->length = length + OSPF6_HEADER_SIZE;