From 333d3fe1c652aeb9aa667c9ff633a1b258593f48 Mon Sep 17 00:00:00 2001 From: Sukchan Lee Date: Fri, 16 Aug 2024 16:39:54 +0900 Subject: [PATCH] clang scan-build static analysis findings/resolutions (#3387) The clang scan-build procedure ``` Assume Ubuntu docker container with open5gs mounted to /src. Assume these tools are installed to docker container: sudo apt install -y clang-tools clang For easy reference to clang scan-build tool: Put normal open5gs build procedure into a file called /src/build ======================= Inside docker container: ======================= export CLANG_OUT_DIR=/src/scan_build_results scan-build -disable-checker deadcode.DeadStores --override-compiler --keep-going --exclude subprojects --exclude tests --exclude lib/asn1c -maxloop 200 -o $CLANG_OUT_DIR -plist-html /src/build 2>&1 | tee /src/logclang.txt ======================= Results: ======================= Results are in html format in $CLANG_OUT_DIR - top level index.html ``` Note that in this analysis the following suppressions were assumed: - no deadcode.DeadStores analysis since those are not functional findings - exclude lib/asn1c for reason that is outside of open5gs control - exclude tests for reason that those are not functional findings - exclude subprojects since those are outside of open5gs control --- lib/core/abts.c | 22 +++++++++++++++---- lib/crypt/ecc.c | 10 +++++++++ lib/crypt/kasumi.c | 12 +++++++++++ lib/crypt/ogs-aes.c | 12 +++++++++++ lib/gtp/v1/types.c | 6 ++++++ lib/ipfw/dummynet.c | 4 ++++ lib/ipfw/ipfw2.c | 36 +++++++++++++++++++++++++++++-- lib/ipfw/tables.c | 47 +++++++++++++++++++++++++++++++++++------ lib/sbi/conv.c | 7 ++++++ lib/sctp/ogs-lksctp.c | 4 ++++ src/amf/nsmf-handler.c | 10 +++++---- src/mme/mme-fd-path.c | 17 ++++++++++++++- src/mme/mme-sm.c | 5 +++++ src/mme/s1ap-handler.c | 4 ++++ src/mme/sgsap-handler.c | 5 +++++ src/nssf/context.c | 4 ++++ src/pcf/pcf-sm.c | 6 ++++-- src/sgwc/context.c | 3 ++- src/sgwc/pfcp-sm.c | 3 +++ src/sgwc/sxa-handler.c | 2 ++ src/smf/gn-build.c | 11 ++++++++++ src/smf/gy-path.c | 10 +++++++++ src/udm/nudm-handler.c | 3 ++- 23 files changed, 222 insertions(+), 21 deletions(-) diff --git a/lib/core/abts.c b/lib/core/abts.c index a81e6b870..86f05a14c 100644 --- a/lib/core/abts.c +++ b/lib/core/abts.c @@ -152,10 +152,17 @@ abts_suite *abts_add_suite(abts_suite *suite, const char *suite_name_full) suite = malloc(sizeof(*suite)); suite->head = subsuite; suite->tail = subsuite; - } - else { - suite->tail->next = subsuite; - suite->tail = subsuite; + } else { + /* Clang scan-build SA: NULL pointer dereference: add check for suite->tail=NULL */ + if (suite->tail) { + suite->tail->next = subsuite; + suite->tail = subsuite; + } else { + fprintf(stderr, "suite->tail=NULL\n"); + fflush(stderr); + free(subsuite); + return(NULL); + } } if (!should_test_run(subsuite->name)) { @@ -203,6 +210,13 @@ static int report(abts_suite *suite) end_suite(suite); } + /* Clang scan-build SA: NULL pointer dereference: suite=NULL */ + if (!suite) { + fprintf(stderr, "suite=NULL\n"); + fflush(stderr); + return(1); + } + for (dptr = suite->head; dptr; dptr = dptr->next) { count += dptr->failed; } diff --git a/lib/crypt/ecc.c b/lib/crypt/ecc.c index 411abe923..9066a21ad 100644 --- a/lib/crypt/ecc.c +++ b/lib/crypt/ecc.c @@ -1075,6 +1075,11 @@ int ecc_make_key(uint8_t p_publicKey[ECC_BYTES+1], uint8_t p_privateKey[ECC_BYTE EccPoint l_public; unsigned l_tries = 0; + /* Clang scan-build SA: Branch condition evaluates to garbage value: In 1st pass thru the do loop the struct l_public + * will not contain a value in the while() check if vli_isZero(l_private)==true and the continue branch is taken. + * Initialize l_public to fix the issue. */ + memset(&l_public, 0, sizeof(EccPoint)); + do { if(!getRandomNumber(l_private) || (l_tries++ >= MAX_TRIES)) @@ -1255,6 +1260,11 @@ int ecdsa_sign(const uint8_t p_privateKey[ECC_BYTES], const uint8_t p_hash[ECC_B EccPoint p; unsigned l_tries = 0; + /* Clang scan-build SA: Branch condition evaluates to garbage value: In 1st pass thru the do loop the struct "p" + * will not contain a value in the while() check if vli_isZero(k)==true and the continue branch is taken. + * Initialize "p" to fix the issue. */ + memset(&p, 0, sizeof(EccPoint)); + do { if(!getRandomNumber(k) || (l_tries++ >= MAX_TRIES)) diff --git a/lib/crypt/kasumi.c b/lib/crypt/kasumi.c index 7ce7fed04..14cc86cad 100644 --- a/lib/crypt/kasumi.c +++ b/lib/crypt/kasumi.c @@ -292,7 +292,13 @@ void kasumi_f8(u8 *key, u32 count, u32 bearer, u32 dir, u8 *data, int length) /* Construct the modified key and then "kasumi" A */ for( n=0; n<16; ++n ) ModKey[n] = (u8)(key[n] ^ 0x55); + + /* Clang scan-build SA: Result of operation is garbage: The function kasumi_key_schedule() is reporting that + * the array parameter "k" (ModKey) has garbage/uninitialized values. Don't see how that is possible + * because the array is fully populated by the loop above. */ +#ifndef __clang_analyzer__ kasumi_key_schedule( ModKey ); +#endif kasumi( A.b8 ); /* First encryption to create modifier */ @@ -454,7 +460,13 @@ u8 *kasumi_f9(u8 *key, u32 count, u32 fresh, u32 dir, u8 *data, int length) * key XORd with 0xAAAA..... */ for( n=0; n<16; ++n ) ModKey[n] = (u8)*key++ ^ 0xAA; + + /* Clang scan-build SA: Result of operation is garbage: The function kasumi_key_schedule() is reporting that + * the array parameter "k" (ModKey) has garbage/uninitialized values. Don't see how that is possible + * because the array is fully populated by the loop above. */ +#ifndef __clang_analyzer__ kasumi_key_schedule( ModKey ); +#endif kasumi( B.b8 ); /* We return the left-most 32-bits of the result */ diff --git a/lib/crypt/ogs-aes.c b/lib/crypt/ogs-aes.c index 382a90537..d8c4c58ce 100644 --- a/lib/crypt/ogs-aes.c +++ b/lib/crypt/ogs-aes.c @@ -1255,6 +1255,12 @@ int ogs_aes_cbc_encrypt(const uint8_t *key, const uint32_t keybits, *outlen = ((inlen - 1) / OGS_AES_BLOCK_SIZE + 1) * OGS_AES_BLOCK_SIZE; + /* Clang scan-build SA: Result of operation is garbage: The function ogs_aes_encrypt() is reporting that the + * array parameter rk has garbage/uninitialized values. The garbage values are because the SA is taking a path + * through ogs_aes_setup_enc() that doesn't match a valid keybits value and therefore the function is not + * populating rk. Fix the issue by initializing rk to 0 here. */ + memset(rk, 0, sizeof(rk)); + nrounds = ogs_aes_setup_enc(rk, key, keybits); while (len >= OGS_AES_BLOCK_SIZE) @@ -1310,6 +1316,12 @@ int ogs_aes_cbc_decrypt(const uint8_t *key, const uint32_t keybits, *outlen = inlen; + /* Clang scan-build SA: Result of operation is garbage: The function ogs_aes_decrypt() is reporting that the + * array parameter rk has garbage/uninitialized values. The garbage values are because the SA is taking a path + * through ogs_aes_setup_enc() (from ogs_aes_setup_dec()) that doesn't match a valid keybits value and + * therefore the function is not populating rk. Fix the issue by initializing rk to 0 here. */ + memset(rk, 0, sizeof(rk)); + nrounds = ogs_aes_setup_dec(rk, key, keybits); if (in != out) diff --git a/lib/gtp/v1/types.c b/lib/gtp/v1/types.c index ba812d7a7..78a0d0159 100644 --- a/lib/gtp/v1/types.c +++ b/lib/gtp/v1/types.c @@ -921,6 +921,12 @@ int ogs_gtp1_parse_pdp_context( CHECK_SPACE_ERR(1 + *ptr); rv = ogs_fqdn_parse(decoded->apn, (const char *)ptr + 1, ogs_min(*ptr, sizeof(decoded->apn))); + /* Clang scan-build SA: Value stored is not used: add check for rv error. */ + if (rv <= 0) { + ogs_error("ogs_fqdn_parse() failed"); + return OGS_ERROR; + } + ptr += 1 + *ptr; CHECK_SPACE_ERR(2); diff --git a/lib/ipfw/dummynet.c b/lib/ipfw/dummynet.c index f31367503..6f56899d4 100644 --- a/lib/ipfw/dummynet.c +++ b/lib/ipfw/dummynet.c @@ -853,6 +853,10 @@ ipfw_config_pipe(int ac, char **av) fs->fs_nr = i + DN_MAX_ID; fs->sched_nr = i; break; + default: + /* Clang scan-build SA: NULL pointer dereference: missing "default" case leaves fs=NULL. */ + ogs_error("unrecognised option %d", co.do_pipe); + break; } /* set to -1 those fields for which we want to reuse existing * values from the kernel. diff --git a/lib/ipfw/ipfw2.c b/lib/ipfw/ipfw2.c index 9711b9c49..2675bf451 100644 --- a/lib/ipfw/ipfw2.c +++ b/lib/ipfw/ipfw2.c @@ -764,6 +764,8 @@ print_flags_buffer(char *buf, size_t sz, struct _s_x *list, uint32_t set) int _substrcmp(const char *str1, const char* str2) { + /* Clang scan-build SA: Argument with nonnull attribute passed null. */ + if ((!str1) || (!str2)) return(1); if (strncmp(str1, str2, strlen(str1)) != 0) return 1; @@ -2464,9 +2466,15 @@ list_dyn_range(struct cmdline_opts *co, struct format_opts *fo, void ipfw_list(int ac, char *av[], int show_counters) { + /* Clang scan-build SA: Uninitialized argument value: false-positive report for the variable + * sz being uninitialized if ipfw_get_config() doesn't fill in sz and returns an error. + * ipfw_get_config() only returns success if sz is filled in. The SA is incorrectly creating + * a path where ipfw_get_config() doesn't fill in sz on an error but the SA is using + * error=0 (success) below to pass an unitialized sz to ipfw_show_config(). + * Initialize sz=0 to make the SA happy. */ ipfw_cfg_lheader *cfg; struct format_opts sfo; - size_t sz; + size_t sz = 0; int error; int lac; char **lav; @@ -2535,6 +2543,12 @@ ipfw_show_config(struct cmdline_opts *co, struct format_opts *fo, ipfw_obj_ctlv *ctlv, *tstate; ipfw_obj_tlv *rbase; + /* Clang scan-build SA: NULL pointer dereference */ + if (!cfg) { + ogs_error("!cfg"); + return(EX_DATAERR); + } + /* * Handle tablenames TLV first, if any */ @@ -2853,7 +2867,10 @@ fill_ip(ipfw_insn_ip *cmd, char *av, int cblen, struct tidx *tstate) d[1] = htonl(~0 << (32 - masklen)); break; case '{': /* no mask, assume /24 and put back the '{' */ - d[1] = htonl(~0 << (32 - 24)); + /* Clang scan-build SA: Result of operation is garbage: The SA is whining that the result of the << is + * undefined because the left operand (~0) is negative. Fix by casting to unsigned. Why is this + * the only place the SA reports this issue? Same code a few lines above... */ + d[1] = htonl((uint32_t)(~0) << (32 - 24)); *(--p) = md; break; @@ -4914,6 +4931,14 @@ ipfw_get_tracked_ifaces(ipfw_obj_lheader **polh) } sz = req.size; +#ifndef __clang_analyzer__ + /* Clang scan-build SA: Memory error - use of 0 allocated: This is a code bug or a false-positive that is + * not clear. do_get3(..., &req.opheader, &sz) calls getsockopt(..., optval=&req.opheader, optlen=&sz) + * which fills in optval & optlen on return. However opheader does not contain a size field and + * sz (optlen) is overwritten by the line above. req.size appears to still be 0 from the memset() at the + * top of the function. This looks like a bug but hard to believe because this is code from/for a BSD + * linux firewall package. */ + if ((olh = calloc(1, sz)) == NULL) return (ENOMEM); @@ -4924,6 +4949,7 @@ ipfw_get_tracked_ifaces(ipfw_obj_lheader **polh) } *polh = olh; +#endif return (0); } @@ -4954,6 +4980,12 @@ ipfw_list_tifaces(void) err(EX_OSERR, "Unable to request ipfw tracked interface list"); + /* Clang scan-build SA: NULL pointer dereference: false-positive report that olh=NULL after + * ipfw_get_tracked_ifaces()=0 (2 functions up). This is incorrect because ipfw_get_tracked_ifaces() + * only returns 0 when it sets the olh pointer. But add an assert just in case and to stop the SA from + * reporting this. */ + ogs_assert(olh); + qsort(olh + 1, olh->count, olh->objsize, ifinfo_cmp); info = (ipfw_iface_info *)(olh + 1); diff --git a/lib/ipfw/tables.c b/lib/ipfw/tables.c index e75b59ae4..87690e7a7 100644 --- a/lib/ipfw/tables.c +++ b/lib/ipfw/tables.c @@ -263,9 +263,16 @@ ipfw_table_handler(int ac, char *av[]) case TOK_INFO: arg = (tcmd == TOK_DETAIL) ? (void *)1 : NULL; if (is_all == 0) { + /* Clang scan-build SA: Uninitialized argument value: false-positive report for the contents + * of "i" being undefined if table_get_info() doesn't fill in "i" and returns an error. + * table_get_info() only returns success if "i" is filled in. The SA is incorrectly creating + * a path where table_get_info() doesn't fill in "i" on an error but the SA is using + * error=0 (success) below to pass an uninitialized "i" to table_show_info(). */ +#ifndef __clang_analyzer__ if ((error = table_get_info(&oh, &i)) != 0) err(EX_OSERR, "failed to request table info"); table_show_info(&i, arg); +#endif } else { error = tables_foreach(table_show_info, arg, 1); if (error != 0) @@ -275,9 +282,16 @@ ipfw_table_handler(int ac, char *av[]) case TOK_LIST: if (is_all == 0) { ipfw_xtable_info i; + /* Clang scan-build SA: Result of operation is garbage: false-positive report for the contents + * of "i" being undefined if table_get_info() doesn't fill in "i" and returns an error. + * table_get_info() only returns success if "i" is filled in. The SA is incorrectly creating + * a path where table_get_info() doesn't fill in "i" on an error but the SA is using + * error=0 (success) below to pass an uninitialized "i" to table_show_one(). */ +#ifndef __clang_analyzer__ if ((error = table_get_info(&oh, &i)) != 0) err(EX_OSERR, "failed to request table info"); table_show_one(&i, NULL); +#endif } else { error = tables_foreach(table_show_one, NULL, 1); if (error != 0) @@ -1588,6 +1602,14 @@ tables_foreach(table_cb_t *f, void *arg, int sort) static int table_do_get_list(ipfw_xtable_info *i, ipfw_obj_header **poh) { + /* Clang scan-build SA: Memory error - use of 0 allocated: This is a code bug or a false-positive that + * is not clear. The SA is taking a path that assumes i->size=0 on 1st pass thru the for loop & + * therefore the "sz < i->size" check fails and sz=0 remains from the initialization at the top of + * the function for the calloc() call. "i->size=0" would be from tables_foreach(). In that + * function ipfw_xtable_info is contained within a larger buffer that is init'ed by calloc(). It + * is not obvious how ipfw_xtable_info->size is getting set to a >0 value before getting passed + * to this function. */ +#ifndef __clang_analyzer__ ipfw_obj_header *oh; size_t sz; int c; @@ -1612,6 +1634,7 @@ table_do_get_list(ipfw_xtable_info *i, ipfw_obj_header **poh) break; } free(oh); +#endif return (errno); } @@ -1626,6 +1649,12 @@ table_show_list(ipfw_obj_header *oh, int need_header) uint32_t count; ipfw_xtable_info *i; + /* Clang scan-build SA: NULL pointer dereference: oh=NULL. */ + if (!oh) { + ogs_error("Unable to print table list, oh=NULL"); + return; + } + i = (ipfw_xtable_info *)(oh + 1); tent = (ipfw_obj_tentry *)(i + 1); @@ -1829,14 +1858,17 @@ table_do_get_vlist(ipfw_obj_lheader **polh) void ipfw_list_ta(int ac, char *av[]) { - ipfw_obj_lheader *olh; + /* Clang scan-build SA: Result of operation is garbage: initialize olh=NULL and check for NULL before use. */ + ipfw_obj_lheader *olh = NULL; ipfw_ta_info *info; int error, i; const char *atype; error = table_do_get_algolist(&olh); - if (error != 0) - err(EX_OSERR, "Unable to request algorithm list"); + if ((error != 0) || (!olh)) { + ogs_error("Unable to request algorithm list"); + return; + } info = (ipfw_ta_info *)(olh + 1); for (i = 0; i < olh->count; i++) { @@ -1890,15 +1922,18 @@ compare_values(const void *_a, const void *_b) void ipfw_list_values(int ac, char *av[]) { - ipfw_obj_lheader *olh; + /* Clang scan-build SA: Result of operation is garbage: initialize olh=NULL and check for NULL before use. */ + ipfw_obj_lheader *olh = NULL; struct _table_value *v; int error, i; uint32_t vmask; char buf[128]; error = table_do_get_vlist(&olh); - if (error != 0) - err(EX_OSERR, "Unable to request value list"); + if ((error != 0) || (!olh)) { + ogs_error("Unable to request value list"); + return; + } vmask = 0x7FFFFFFF; /* Similar to IPFW_VTYPE_LEGACY */ diff --git a/lib/sbi/conv.c b/lib/sbi/conv.c index e5a83fb89..353a6a07a 100644 --- a/lib/sbi/conv.c +++ b/lib/sbi/conv.c @@ -94,6 +94,13 @@ char *ogs_supi_from_suci(char *suci) return NULL; } + /* Clang scan-build SA: Branch condition evaluates to a garbage value: If array "array" is not fully populated + * in the while loop below then later access in the following switch-case may check uninitialized values. + * Initialize "array" to NULL pointers to fix the issue. */ + for (i = 0; i < MAX_SUCI_TOKEN; i++) { + array[i] = NULL; + } + p = tmp; i = 0; while((array[i++] = strsep(&p, "-"))) { diff --git a/lib/sctp/ogs-lksctp.c b/lib/sctp/ogs-lksctp.c index a0548229d..89a6b985c 100644 --- a/lib/sctp/ogs-lksctp.c +++ b/lib/sctp/ogs-lksctp.c @@ -194,6 +194,10 @@ int ogs_sctp_connect(ogs_sock_t *sock, ogs_sockaddr_t *sa_list) ogs_assert(sock); + /* Clang scan-build SA: NULL pointer dereference: if addr=sa_list=NULL then the macro OGS_PORT(sa_list) will + * dereference the NULL pointer. */ + ogs_assert(sa_list); + addr = sa_list; while (addr) { if (ogs_sock_connect(sock, addr) == OGS_OK) { diff --git a/src/amf/nsmf-handler.c b/src/amf/nsmf-handler.c index da6538c10..6b7aa8324 100644 --- a/src/amf/nsmf-handler.c +++ b/src/amf/nsmf-handler.c @@ -377,8 +377,9 @@ int amf_nsmf_pdusession_handle_update_sm_context( case OpenAPI_n2_sm_info_type_PDU_RES_MOD_REQ: if (!n1smbuf) { - ogs_error("[%s:%d] No N1 SM Content [%s]", - amf_ue->supi, sess->psi, n1SmMsg->content_id); + /* Clang scan-build SA: NULL pointer deference: n1SmMsg=NULL, remove logging of n1SmMsg->content_id. */ + ogs_error("[%s:%d] No N1 SM Content", + amf_ue->supi, sess->psi); r = nas_5gs_send_back_gsm_message(ran_ue, sess, OGS_5GMM_CAUSE_PAYLOAD_WAS_NOT_FORWARDED, AMF_NAS_BACKOFF_TIME); @@ -419,8 +420,9 @@ int amf_nsmf_pdusession_handle_update_sm_context( case OpenAPI_n2_sm_info_type_PDU_RES_REL_CMD: if (!n1smbuf) { - ogs_error("[%s:%d] No N1 SM Content [%s]", - amf_ue->supi, sess->psi, n1SmMsg->content_id); + /* Clang scan-build SA: NULL pointer deference: n1SmMsg=NULL, remove logging of n1SmMsg->content_id. */ + ogs_error("[%s:%d] No N1 SM Content", + amf_ue->supi, sess->psi); r = nas_5gs_send_back_gsm_message(ran_ue, sess, OGS_5GMM_CAUSE_PAYLOAD_WAS_NOT_FORWARDED, AMF_NAS_BACKOFF_TIME); diff --git a/src/mme/mme-fd-path.c b/src/mme/mme-fd-path.c index 9983545ca..3e159e001 100644 --- a/src/mme/mme-fd-path.c +++ b/src/mme/mme-fd-path.c @@ -146,6 +146,8 @@ static int mme_s6a_subscription_data_from_avp(struct avp *avp, ogs_assert(ret == 0); if (avpch1) { ret = fd_msg_avp_hdr(avpch1, &hdr); + /* Clang scan-build SA: Value stored is not used: add ogs_assert(). */ + ogs_assert(ret == 0); ogs_ascii_to_hex( (char*)hdr->avp_value->os.data, (int)hdr->avp_value->os.len, buf, sizeof(buf)); @@ -292,6 +294,8 @@ static int mme_s6a_subscription_data_from_avp(struct avp *avp, ogs_assert(ret == 0); if (avpch3) { ret = fd_msg_avp_hdr(avpch3, &hdr); + /* Clang scan-build SA: Value stored is not used: add ogs_assert(). */ + ogs_assert(ret == 0); session->name = ogs_strndup( (char*)hdr->avp_value->os.data, hdr->avp_value->os.len); @@ -565,6 +569,8 @@ static int mme_s6a_subscription_data_from_avp(struct avp *avp, ogs_assert(ret == 0); while (avpch4) { ret = fd_msg_avp_hdr(avpch4, &hdr); + /* Clang scan-build SA: Value stored is not used: add ogs_assert(). */ + ogs_assert(ret == 0); switch(hdr->avp_code) { case OGS_DIAM_S6A_AVP_CODE_MIP_HOME_AGENT_ADDRESS: ret = fd_msg_avp_value_interpret(avpch4, @@ -1018,8 +1024,12 @@ static void mme_s6a_aia_cb(void *data, struct msg **msg) ret = fd_avp_search_avp(avp_e_utran_vector, ogs_diam_s6a_rand, &avp_rand); + /* Clang scan-build SA: Value stored is not used: add ogs_assert(). */ + ogs_assert(ret == 0); if (avp) { ret = fd_msg_avp_hdr(avp_rand, &hdr); + /* Clang scan-build SA: Value stored is not used: add ogs_assert(). */ + ogs_assert(ret == 0); memcpy(e_utran_vector->rand, hdr->avp_value->os.data, ogs_min(hdr->avp_value->os.len, OGS_ARRAY_SIZE(e_utran_vector->rand))); @@ -1440,6 +1450,8 @@ static void mme_s6a_ula_cb(void *data, struct msg **msg) uint32_t subdatamask = 0; ret = mme_s6a_subscription_data_from_avp(avp, subscription_data, mme_ue, &subdatamask); + /* Clang scan-build SA: Value stored is not used: add ogs_assert(). */ + ogs_assert(ret == 0); if (!(subdatamask & OGS_DIAM_S6A_SUBDATA_NAM)) { mme_ue->network_access_mode = 0; @@ -1999,7 +2011,8 @@ static int mme_ogs_diam_s6a_idr_cb( struct msg **msg, struct avp *avp, int ret; char imsi_bcd[OGS_MAX_IMSI_BCD_LEN+1]; uint32_t result_code = 0; - bool has_subscriber_data; + /* Clang scan-build SA: Branch condition evaluates to a garbage value: has_subscriber_data can be used uninitialized. */ + bool has_subscriber_data = false; struct msg *ans, *qry; @@ -2064,6 +2077,8 @@ static int mme_ogs_diam_s6a_idr_cb( struct msg **msg, struct avp *avp, uint32_t subdatamask = 0; ret = mme_s6a_subscription_data_from_avp(avp, subscription_data, mme_ue, &subdatamask); + /* Clang scan-build SA: Value stored is not used: add ogs_assert(). */ + ogs_assert(ret == 0); idr_message->subdatamask = subdatamask; ogs_info("[%s] Subscription-Data Processed.", imsi_bcd); } diff --git a/src/mme/mme-sm.c b/src/mme/mme-sm.c index e83f896b8..074dd2cbe 100644 --- a/src/mme/mme-sm.c +++ b/src/mme/mme-sm.c @@ -854,6 +854,11 @@ cleanup: mme_gn_handle_sgsn_context_request(xact, >p1_message.sgsn_context_request); break; case OGS_GTP1_SGSN_CONTEXT_RESPONSE_TYPE: + /* Clang scan-build SA: NULL pointer dereference: mme_ue=NULL if both gtp1_message.h.teid=0 and + * xact->local_teid=0. The following function mme_gn_handle_sgsn_context_response() handles the NULL + * but the later calls to OGS_FSM_TRAN() to change state will be a NULL pointer dereference. */ + ogs_assert(mme_ue); + /* 3GPP TS 23.401 Figure D.3.6-1 step 5 */ rv = mme_gn_handle_sgsn_context_response(xact, mme_ue, >p1_message.sgsn_context_response); if (rv == OGS_GTP1_CAUSE_ACCEPT) { diff --git a/src/mme/s1ap-handler.c b/src/mme/s1ap-handler.c index e8063ecf8..62fc0895b 100644 --- a/src/mme/s1ap-handler.c +++ b/src/mme/s1ap-handler.c @@ -2268,6 +2268,10 @@ void s1ap_handle_enb_direct_information_transfer( } } + /* Clang scan-build SA: NULL pointer dereference: Inter_SystemInformationTransferType=NULL if above + * protocolIEs.list.count=0 in loop. */ + ogs_assert(Inter_SystemInformationTransferType); + RIMTransfer = Inter_SystemInformationTransferType->choice.rIMTransfer; RIMInformation = &RIMTransfer->rIMInformation; diff --git a/src/mme/sgsap-handler.c b/src/mme/sgsap-handler.c index d9b93b88c..3779bac13 100644 --- a/src/mme/sgsap-handler.c +++ b/src/mme/sgsap-handler.c @@ -124,6 +124,11 @@ void sgsap_handle_location_update_accept(mme_vlr_t *vlr, ogs_pkbuf_t *pkbuf) return; error: + /* Clang scan-build SA: NULL pointer dereference: mme_ue=NULL if root=NULL. */ + if (!mme_ue) { + ogs_error("!mme_ue"); + return; + } r = nas_eps_send_attach_reject( enb_ue_find_by_id(mme_ue->enb_ue_id), mme_ue, OGS_NAS_EMM_CAUSE_PROTOCOL_ERROR_UNSPECIFIED, diff --git a/src/nssf/context.c b/src/nssf/context.c index d776926f2..ae472b27f 100644 --- a/src/nssf/context.c +++ b/src/nssf/context.c @@ -233,6 +233,10 @@ int nssf_context_parse_config(void) addr, addr6, port, &h); ogs_assert(nrf_id); + /* Clang scan-build SA: Argument with nonnull attribute passed null: + * sst may be NULL in atoi(sst) if the "uri" key path is followed. */ + ogs_assert(sst); + nsi = nssf_nsi_add( nrf_id, atoi(sst), diff --git a/src/pcf/pcf-sm.c b/src/pcf/pcf-sm.c index 7354c67d3..700320500 100644 --- a/src/pcf/pcf-sm.c +++ b/src/pcf/pcf-sm.c @@ -280,8 +280,9 @@ void pcf_state_operational(ogs_fsm_t *s, pcf_event_t *e) e->h.sbi.message = &message; ogs_fsm_dispatch(&sess->sm, e); if (OGS_FSM_CHECK(&sess->sm, pcf_sm_state_exception)) { + /* Clang scan-build SA: NULL pointer dereference: pcf_ue=NULL, remove logging of pcf_ue->supi. */ ogs_error("[%s:%d] State machine exception", - pcf_ue->supi, sess->psi); + pcf_ue ? pcf_ue->supi : "Unknown", sess->psi); pcf_sess_remove(sess); } break; @@ -331,8 +332,9 @@ void pcf_state_operational(ogs_fsm_t *s, pcf_event_t *e) e->h.sbi.message = &message; ogs_fsm_dispatch(&sess->sm, e); if (OGS_FSM_CHECK(&sess->sm, pcf_sm_state_exception)) { + /* Clang scan-build SA: NULL pointer dereference: pcf_ue=NULL, remove logging of pcf_ue->supi. */ ogs_error("[%s:%d] State machine exception", - pcf_ue->supi, sess->psi); + pcf_ue ? pcf_ue->supi : "Unknown", sess->psi); pcf_sess_remove(sess); } break; diff --git a/src/sgwc/context.c b/src/sgwc/context.c index 9ab44cd88..7d7e11de3 100644 --- a/src/sgwc/context.c +++ b/src/sgwc/context.c @@ -163,7 +163,8 @@ int sgwc_context_parse_config(void) sgwc_ue_t *sgwc_ue_add_by_message(ogs_gtp2_message_t *message) { sgwc_ue_t *sgwc_ue = NULL; - ogs_gtp2_create_session_request_t *req = &message->create_session_request; + /* Clang scan-build SA: Dead initialization: Don't set req before message is checked for NULL. */ + ogs_gtp2_create_session_request_t *req; ogs_assert(message); diff --git a/src/sgwc/pfcp-sm.c b/src/sgwc/pfcp-sm.c index c23f77cd6..e0357ae61 100644 --- a/src/sgwc/pfcp-sm.c +++ b/src/sgwc/pfcp-sm.c @@ -302,6 +302,9 @@ void sgwc_pfcp_state_associated(ogs_fsm_t *s, sgwc_event_t *e) } up_f_seid = rsp->up_f_seid.data; ogs_assert(up_f_seid); + /* Clang scan-build SA: NULL pointer dereference: sess=NULL if both message->h.seid=0 and + * xact->local_seid=0. */ + ogs_assert(sess); sess->sgwu_sxa_seid = be64toh(up_f_seid->seid); } else { sgwc_sxa_handle_session_establishment_response( diff --git a/src/sgwc/sxa-handler.c b/src/sgwc/sxa-handler.c index 48ba18447..9fdf72e9c 100644 --- a/src/sgwc/sxa-handler.c +++ b/src/sgwc/sxa-handler.c @@ -865,6 +865,8 @@ void sgwc_sxa_handle_session_modification_response( pgw_s5u_teid.teid = htobe32(ul_tunnel->remote_teid); rv = ogs_gtp2_ip_to_f_teid( &ul_tunnel->remote_ip, &pgw_s5u_teid, &len); + /* Clang scan-build SA: Value stored is not used: add ogs_assert(). */ + ogs_assert(rv == OGS_OK); gtp_rsp->bearer_contexts.s5_s8_u_pgw_f_teid.presence = 1; gtp_rsp->bearer_contexts.s5_s8_u_pgw_f_teid.data = &pgw_s5u_teid; gtp_rsp->bearer_contexts.s5_s8_u_pgw_f_teid.len = len; diff --git a/src/smf/gn-build.c b/src/smf/gn-build.c index a6c12b01a..abdc8e4d0 100644 --- a/src/smf/gn-build.c +++ b/src/smf/gn-build.c @@ -192,8 +192,19 @@ ogs_pkbuf_t *smf_gn_build_create_pdp_context_response( /* End User Address */ rv = ogs_paa_to_ip(&sess->paa, &ip_eua); + /* Clang scan-build SA: Value stored is not used: add check for rv error. */ + if (rv != OGS_OK) { + ogs_error("ogs_paa_to_ip() failed"); + return NULL; + } rv = ogs_gtp1_ip_to_eua(sess->session.session_type, &ip_eua, &eua, &eua_len); + /* Clang scan-build SA: Value stored is not used: add check for rv error. */ + if (rv != OGS_OK) { + ogs_error("ogs_gtp1_ip_to_eua() failed"); + return NULL; + } + rsp->end_user_address.presence = 1; rsp->end_user_address.data = &eua; rsp->end_user_address.len = eua_len; diff --git a/src/smf/gy-path.c b/src/smf/gy-path.c index 997d0c4da..791ab40eb 100644 --- a/src/smf/gy-path.c +++ b/src/smf/gy-path.c @@ -401,11 +401,15 @@ static void fill_ps_information(smf_sess_t *sess, uint32_t cc_request_type, /* PDP-Address, TS 32.299 7.2.137 */ if (sess->ipv4) { ret = fd_msg_avp_new(ogs_diam_gy_pdp_address, 0, &avpch2); + /* Clang scan-build SA: Value stored is not used: add ogs_assert(). */ + ogs_assert(ret == 0); sin.sin_family = AF_INET; memcpy(&sin.sin_addr.s_addr, (uint8_t*)&sess->ipv4->addr[0], OGS_IPV4_LEN); ret = fd_msg_avp_value_encode(&sin, avpch2); ogs_assert(ret == 0); ret = fd_msg_avp_add(avpch1, MSG_BRW_LAST_CHILD, avpch2); + /* Clang scan-build SA: Value stored is not used: add ogs_assert(). */ + ogs_assert(ret == 0); } if (sess->ipv6) { ret = fd_msg_avp_new(ogs_diam_gy_pdp_address, 0, &avpch2); @@ -425,6 +429,8 @@ static void fill_ps_information(smf_sess_t *sess, uint32_t cc_request_type, /* SGSN-Address */ if (sess->sgw_s5c_ip.ipv4) { ret = fd_msg_avp_new(ogs_diam_gy_sgsn_address, 0, &avpch2); + /* Clang scan-build SA: Value stored is not used: add ogs_assert(). */ + ogs_assert(ret == 0); sin.sin_family = AF_INET; memcpy(&sin.sin_addr.s_addr, (uint8_t*)&sess->sgw_s5c_ip.addr, OGS_IPV4_LEN); ret = fd_msg_avp_value_encode(&sin, avpch2); @@ -434,6 +440,8 @@ static void fill_ps_information(smf_sess_t *sess, uint32_t cc_request_type, } if (sess->sgw_s5c_ip.ipv6) { ret = fd_msg_avp_new(ogs_diam_gy_sgsn_address, 0, &avpch2); + /* Clang scan-build SA: Value stored is not used: add ogs_assert(). */ + ogs_assert(ret == 0); sin6.sin6_family = AF_INET6; memcpy(&sin6.sin6_addr.s6_addr, (uint8_t*)&sess->sgw_s5c_ip.addr6[0], OGS_IPV6_LEN); ret = fd_msg_avp_value_encode(&sin6, avpch2); @@ -569,6 +577,8 @@ static void fill_ps_information(smf_sess_t *sess, uint32_t cc_request_type, if (smf_ue->imeisv_len > 0) { /* User-Equipment-Info, 3GPP TS 32.299 7.1.17 */ ret = fd_msg_avp_new(ogs_diam_gy_user_equipment_info, 0, &avpch2); + /* Clang scan-build SA: Value stored is not used: add ogs_assert(). */ + ogs_assert(ret == 0); /* User-Equipment-Info-Type 0 (IMEI) */ ret = fd_msg_avp_new(ogs_diam_gy_user_equipment_info_type, 0, &avpch3); diff --git a/src/udm/nudm-handler.c b/src/udm/nudm-handler.c index 45b3848c0..8ce08e7fa 100644 --- a/src/udm/nudm-handler.c +++ b/src/udm/nudm-handler.c @@ -690,7 +690,8 @@ bool udm_nudm_sdm_handle_subscription_create( return false; } - if ((!SDMSubscription->monitored_resource_uris) && + /* Clang scan-build SA: NULL pointer dereference: change && to || in case monitored_resource_uris=NULL. */ + if ((!SDMSubscription->monitored_resource_uris) || (!SDMSubscription->monitored_resource_uris->count)) { ogs_error("[%s] No monitoredResourceUris", udm_ue->supi); ogs_assert(true ==