From b3ac5b35ebbb67e3d760e806b1a963f8815bae2f Mon Sep 17 00:00:00 2001 From: Pau Espin Pedrol Date: Tue, 18 Jan 2022 03:23:40 +0100 Subject: [PATCH] [SMF] Fix potential null pointer dereference (#1324) * [SMF] Fix potential null pointer dereference Pointer "sess" was first dereferenced and later on checked for nullness. This is clearly wrong. Rearrange the code path to make sure the check is done first, then dereferenced. * gitignore: Add subprojects/libtins * cosmetic: Fix whitespace --- .gitignore | 3 ++- lib/gtp/context.h | 4 ++-- lib/gtp/xact.h | 12 +++++------ src/smf/context.c | 22 ++++++++++---------- src/smf/gtp-path.c | 2 +- src/smf/meson.build | 4 ++-- src/smf/s5c-handler.c | 48 ++++++++++++++++++++----------------------- 7 files changed, 46 insertions(+), 49 deletions(-) diff --git a/.gitignore b/.gitignore index 4f9427258..1783da8be 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ # This directory is fetched during first build and is present in this directory subprojects/freeDiameter +subprojects/libtins subprojects/usrsctp -webui/.next \ No newline at end of file +webui/.next diff --git a/lib/gtp/context.h b/lib/gtp/context.h index 552b47eea..a49ebe05f 100644 --- a/lib/gtp/context.h +++ b/lib/gtp/context.h @@ -73,8 +73,8 @@ typedef struct ogs_gtp_node_s { ogs_ip_t ip; /* F-TEID IP Address Duplicate Check */ - ogs_list_t local_list; - ogs_list_t remote_list; + ogs_list_t local_list; + ogs_list_t remote_list; } ogs_gtp_node_t; typedef struct ogs_gtpu_resource_s { diff --git a/lib/gtp/xact.h b/lib/gtp/xact.h index 94c85f6a3..d96f1be0f 100644 --- a/lib/gtp/xact.h +++ b/lib/gtp/xact.h @@ -28,9 +28,9 @@ extern "C" { #endif -/* +/* * p225-226 Chapter 7.6 in TS 29.274 V15.9.0 - * + * * A Sequence Number used for a Command message shall have the most significant * bit set to 1. A Sequence Number in a message, which was triggered by * a Command message, as well as respective Triggered Reply message @@ -54,10 +54,10 @@ extern "C" { typedef struct ogs_gtp_xact_s { ogs_lnode_t node; /**< A node of list */ ogs_index_t index; - + #define OGS_GTP_LOCAL_ORIGINATOR 0 #define OGS_GTP_REMOTE_ORIGINATOR 1 - uint8_t org; /**< Transaction' originator. + uint8_t org; /**< Transaction' originator. local or remote */ uint32_t xid; /**< Transaction ID */ @@ -67,8 +67,8 @@ typedef struct ogs_gtp_xact_s { void *data; /**< Transaction Data */ int step; /**< Current step in the sequence. - 1 : Initial - 2 : Triggered + 1 : Initial + 2 : Triggered 3 : Triggered-Reply */ struct { uint8_t type; /**< Message type history */ diff --git a/src/smf/context.c b/src/smf/context.c index 6e585913b..ba575c217 100644 --- a/src/smf/context.c +++ b/src/smf/context.c @@ -191,7 +191,7 @@ int smf_context_parse_config(void) const char *smf_key = ogs_yaml_iter_key(&smf_iter); ogs_assert(smf_key); if (!strcmp(smf_key, "freeDiameter")) { - yaml_node_t *node = + yaml_node_t *node = yaml_document_get_node(document, smf_iter.pair->value); ogs_assert(node); if (node->type == YAML_SCALAR_NODE) { @@ -204,10 +204,10 @@ int smf_context_parse_config(void) const char *fd_key = ogs_yaml_iter_key(&fd_iter); ogs_assert(fd_key); if (!strcmp(fd_key, "identity")) { - self.diam_config->cnf_diamid = + self.diam_config->cnf_diamid = ogs_yaml_iter_value(&fd_iter); } else if (!strcmp(fd_key, "realm")) { - self.diam_config->cnf_diamrlm = + self.diam_config->cnf_diamrlm = ogs_yaml_iter_value(&fd_iter); } else if (!strcmp(fd_key, "port")) { const char *v = ogs_yaml_iter_value(&fd_iter); @@ -216,7 +216,7 @@ int smf_context_parse_config(void) const char *v = ogs_yaml_iter_value(&fd_iter); if (v) self.diam_config->cnf_port_tls = atoi(v); } else if (!strcmp(fd_key, "listen_on")) { - self.diam_config->cnf_addr = + self.diam_config->cnf_addr = ogs_yaml_iter_value(&fd_iter); } else if (!strcmp(fd_key, "no_fwd")) { self.diam_config->cnf_flags.no_fwd = @@ -299,7 +299,7 @@ int smf_context_parse_config(void) ogs_yaml_iter_key(&conn_iter); ogs_assert(conn_key); if (!strcmp(conn_key, "identity")) { - identity = + identity = ogs_yaml_iter_value(&conn_iter); } else if (!strcmp(conn_key, "addr")) { addr = @@ -1041,7 +1041,7 @@ smf_sess_t *smf_sess_add_by_gtp_message(ogs_gtp_message_t *message) ogs_trace("smf_sess_add_by_message() [APN:%s]", apn); - /* + /* * 7.2.1 in 3GPP TS 29.274 Release 15 * * If the new Create Session Request received by the SMF collides with @@ -1396,14 +1396,14 @@ void smf_sess_remove(smf_sess_t *sess) int i; smf_ue_t *smf_ue = NULL; smf_event_t e; - + char buf1[OGS_ADDRSTRLEN]; char buf2[OGS_ADDRSTRLEN]; ogs_assert(sess); smf_ue = sess->smf_ue; ogs_assert(smf_ue); - + ogs_info("Removed Session: UE IMSI:[%s] DNN:[%s:%d] IPv4:[%s] IPv6:[%s]", smf_ue->supi ? smf_ue->supi : smf_ue->imsi_bcd, sess->session.name, sess->psi, @@ -2106,7 +2106,7 @@ smf_bearer_t *smf_bearer_find_by_pgw_s5u_teid( smf_bearer_t *smf_bearer_find_by_ebi(smf_sess_t *sess, uint8_t ebi) { smf_bearer_t *bearer = NULL; - + ogs_assert(sess); ogs_list_for_each(&sess->bearer_list, bearer) { @@ -2121,7 +2121,7 @@ smf_bearer_t *smf_bearer_find_by_pcc_rule_name( smf_sess_t *sess, char *pcc_rule_name) { smf_bearer_t *bearer = NULL; - + ogs_assert(sess); ogs_assert(pcc_rule_name); @@ -2333,7 +2333,7 @@ void smf_pf_remove_all(smf_bearer_t *bearer) smf_pf_t *smf_pf_find_by_id(smf_bearer_t *bearer, uint8_t id) { smf_pf_t *pf = NULL; - + ogs_list_for_each(&bearer->pf_list, pf) { if (pf->identifier == id) return pf; } diff --git a/src/smf/gtp-path.c b/src/smf/gtp-path.c index 821341ce8..4cdfcccaf 100644 --- a/src/smf/gtp-path.c +++ b/src/smf/gtp-path.c @@ -240,7 +240,7 @@ int smf_gtp_open(void) ogs_list_for_each(&ogs_gtp_self()->gtpc_list, node) { sock = ogs_gtp_server(node); if (!sock) return OGS_ERROR; - + node->poll = ogs_pollset_add(ogs_app()->pollset, OGS_POLLIN, sock->fd, _gtpv2_c_recv_cb, sock); ogs_assert(node->poll); diff --git a/src/smf/meson.build b/src/smf/meson.build index 803c5553f..c646a4f9a 100644 --- a/src/smf/meson.build +++ b/src/smf/meson.build @@ -75,11 +75,11 @@ libsmf_sources = files(''' pfcp-sm.c gtp-path.c s5c-build.c - s5c-handler.c + s5c-handler.c fd-path.c gx-path.c s6b-path.c - gx-handler.c + gx-handler.c pfcp-path.c n4-build.c n4-handler.c diff --git a/src/smf/s5c-handler.c b/src/smf/s5c-handler.c index 1354d41a6..d577a6aaf 100644 --- a/src/smf/s5c-handler.c +++ b/src/smf/s5c-handler.c @@ -111,30 +111,6 @@ void smf_s5c_handle_create_session_request( cause_value = OGS_GTP_CAUSE_MANDATORY_IE_MISSING; } - switch (sess->gtp_rat_type) { - case OGS_GTP_RAT_TYPE_EUTRAN: - if (req->bearer_contexts_to_be_created. - s5_s8_u_sgw_f_teid.presence == 0) { - ogs_error("No S5/S8 SGW GTP-U TEID"); - cause_value = OGS_GTP_CAUSE_MANDATORY_IE_MISSING; - } - if (req->user_location_information.presence == 0) { - ogs_error("No UE Location Information"); - cause_value = OGS_GTP_CAUSE_MANDATORY_IE_MISSING; - } - break; - case OGS_GTP_RAT_TYPE_WLAN: - if (req->bearer_contexts_to_be_created. - s2b_u_epdg_f_teid_5.presence == 0) { - ogs_error("No S2b ePDG GTP-U TEID"); - cause_value = OGS_GTP_CAUSE_MANDATORY_IE_MISSING; - } - break; - default: - ogs_error("Unknown RAT Type [%d]", req->rat_type.u8); - cause_value = OGS_GTP_CAUSE_MANDATORY_IE_MISSING; - } - if (!sess) { ogs_error("No Context"); cause_value = OGS_GTP_CAUSE_CONTEXT_NOT_FOUND; @@ -143,12 +119,32 @@ void smf_s5c_handle_create_session_request( ogs_error("No Gx Diameter Peer"); cause_value = OGS_GTP_CAUSE_REMOTE_PEER_NOT_RESPONDING; } - - if (sess->gtp_rat_type == OGS_GTP_RAT_TYPE_WLAN) { + switch (sess->gtp_rat_type) { + case OGS_GTP_RAT_TYPE_EUTRAN: + if (req->bearer_contexts_to_be_created. + s5_s8_u_sgw_f_teid.presence == 0) { + ogs_error("No S5/S8 SGW GTP-U TEID"); + cause_value = OGS_GTP_CAUSE_MANDATORY_IE_MISSING; + } + if (req->user_location_information.presence == 0) { + ogs_error("No UE Location Information"); + cause_value = OGS_GTP_CAUSE_MANDATORY_IE_MISSING; + } + break; + case OGS_GTP_RAT_TYPE_WLAN: if (!ogs_diam_app_connected(OGS_DIAM_S6B_APPLICATION_ID)) { ogs_error("No S6b Diameter Peer"); cause_value = OGS_GTP_CAUSE_REMOTE_PEER_NOT_RESPONDING; } + if (req->bearer_contexts_to_be_created. + s2b_u_epdg_f_teid_5.presence == 0) { + ogs_error("No S2b ePDG GTP-U TEID"); + cause_value = OGS_GTP_CAUSE_MANDATORY_IE_MISSING; + } + break; + default: + ogs_error("Unknown RAT Type [%d]", req->rat_type.u8); + cause_value = OGS_GTP_CAUSE_MANDATORY_IE_MISSING; } }