From 35ce855e32d6b26818b97d47e40b9ac9962ad670 Mon Sep 17 00:00:00 2001 From: Sukchan Lee Date: Fri, 6 Mar 2026 10:05:24 +0900 Subject: [PATCH] core/tlv, smf: Harden TLV parsing and validate Bearer Context in CSR Two issues (#4277, #4278) reported crashes caused by malformed or unexpected inputs. In the TLV parser, several ogs_assert() checks could be triggered by malformed TLV blocks, resulting in process termination. These checks are replaced with proper error handling: the parser now logs the error, limits hexdump size, frees allocated TLVs, and returns NULL instead of aborting. In the SMF S5-C Create Session Request handler, additional validation is introduced for Bearer Context handling. The implementation now rejects requests containing multiple Bearer Contexts, missing mandatory fields (EBI or Bearer QoS), duplicate EBI values, or invalid TEID/IP information. Several ogs_assert() calls that could be triggered by malformed messages are also replaced with explicit error handling. These changes prevent crashes caused by malformed TLV blocks or unexpected Bearer Context structures and ensure the SMF rejects such requests gracefully. Issues: #4277, #4278 --- lib/core/ogs-tlv-msg.c | 39 +++++++++++++++++++++------ lib/core/ogs-tlv.c | 4 +-- src/smf/s5c-handler.c | 61 ++++++++++++++++++++++++++++++++++++------ 3 files changed, 86 insertions(+), 18 deletions(-) diff --git a/lib/core/ogs-tlv-msg.c b/lib/core/ogs-tlv-msg.c index bdabce533..824e2dc93 100644 --- a/lib/core/ogs-tlv-msg.c +++ b/lib/core/ogs-tlv-msg.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2019 by Sukchan Lee + * Copyright (C) 2019,2026 by Sukchan Lee * Copyright (C) 2022 by sysmocom - s.f.m.c. GmbH * * This file is part of Open5GS. @@ -829,29 +829,52 @@ static ogs_tlv_t *ogs_tlv_parse_block_desc(uint32_t length, void *data, uint8_t ogs_tlv_t *curr = NULL; root = curr = ogs_tlv_get(); - - ogs_assert(curr); + if (!curr) { + ogs_error("ogs_tlv_parse_block() failed[LEN:%d,MODE:%d] - no tlv", + length, msg_mode); + ogs_log_hexdump(OGS_LOG_ERROR, data, ogs_min(length, 512)); + return NULL; + } pos = tlv_get_element_desc(curr, pos, msg_mode, desc); - - ogs_assert(pos); + if (!pos) { + ogs_error("ogs_tlv_parse_block() failed[LEN:%u,MODE:%u] - parse error", + length, msg_mode); + ogs_log_hexdump(OGS_LOG_ERROR, data, ogs_min(length, 512)); + ogs_tlv_free_all(root); + return NULL; + } while(pos - blk < length) { prev = curr; curr = ogs_tlv_get(); - ogs_assert(curr); + if (!curr) { + ogs_error("ogs_tlv_parse_block() failed[LEN:%d,MODE:%d]", + length, msg_mode); + ogs_log_hexdump(OGS_LOG_ERROR, data, ogs_min(length, 512)); + + ogs_tlv_free_all(root); + return NULL; + } prev->next = curr; pos = tlv_get_element_desc(curr, pos, msg_mode, desc); - ogs_assert(pos); + if (!pos) { + ogs_error("ogs_tlv_parse_block() failed[LEN:%d,MODE:%d]", + length, msg_mode); + ogs_log_hexdump(OGS_LOG_ERROR, data, ogs_min(length, 512)); + + ogs_tlv_free_all(root); + return NULL; + } } if (length != (pos - blk)) { ogs_error("ogs_tlv_parse_block() failed[LEN:%d,MODE:%d]", length, msg_mode); ogs_error("POS[%p] BLK[%p] POS-BLK[%d]", pos, blk, (int)(pos - blk)); - ogs_log_hexdump(OGS_LOG_FATAL, data, length); + ogs_log_hexdump(OGS_LOG_ERROR, data, ogs_min(length, 512)); ogs_tlv_free_all(root); return NULL; diff --git a/lib/core/ogs-tlv.c b/lib/core/ogs-tlv.c index 3bfa77bf8..1e99204e1 100644 --- a/lib/core/ogs-tlv.c +++ b/lib/core/ogs-tlv.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2019 by Sukchan Lee + * Copyright (C) 2019,2026 by Sukchan Lee * Copyright (C) 2022 by sysmocom - s.f.m.c. GmbH * * This file is part of Open5GS. @@ -510,7 +510,7 @@ ogs_tlv_t *ogs_tlv_parse_block(uint32_t length, void *data, uint8_t mode) if (length != (pos - blk)) { ogs_error("ogs_tlv_parse_block() failed[LEN:%d,MODE:%d]", length, mode); ogs_error("POS[%p] BLK[%p] POS-BLK[%d]", pos, blk, (int)(pos - blk)); - ogs_log_hexdump(OGS_LOG_FATAL, data, length); + ogs_log_hexdump(OGS_LOG_ERROR, data, ogs_min(length, 512)); ogs_tlv_free_all(root); return NULL; diff --git a/src/smf/s5c-handler.c b/src/smf/s5c-handler.c index e84523df2..93adfd01d 100644 --- a/src/smf/s5c-handler.c +++ b/src/smf/s5c-handler.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2019-2023 by Sukchan Lee + * Copyright (C) 2019-2026 by Sukchan Lee * * This file is part of Open5GS. * @@ -96,6 +96,7 @@ uint8_t smf_s5c_handle_create_session_request( char buf2[OGS_ADDRSTRLEN]; int i, rv; + int bearer_count; uint8_t cause_value = 0; ogs_gtp2_uli_t uli; @@ -318,9 +319,16 @@ uint8_t smf_s5c_handle_create_session_request( sgw_s5c_teid = req->sender_f_teid_for_control_plane.data; ogs_assert(sgw_s5c_teid); /* sess->sgw_s5c_teid has already been updated in SMF-SM */ - ogs_assert(sess->sgw_s5c_teid == be32toh(sgw_s5c_teid->teid)); + if (sess->sgw_s5c_teid != be32toh(sgw_s5c_teid->teid)) { + ogs_error("SGW-S5C TEID mismatch (sess=0x%x, msg=0x%x)", + sess->sgw_s5c_teid, be32toh(sgw_s5c_teid->teid)); + return OGS_GTP2_CAUSE_CONTEXT_NOT_FOUND; + } rv = ogs_gtp2_f_teid_to_ip(sgw_s5c_teid, &sess->sgw_s5c_ip); - ogs_assert(rv == OGS_OK); + if (rv != OGS_OK) { + ogs_error("Invalid SGW-S5C TEID"); + return OGS_GTP2_CAUSE_MANDATORY_IE_INCORRECT; + } ogs_debug(" SGW_S5C_TEID[0x%x] SMF_N4_TEID[0x%x]", sess->sgw_s5c_teid, sess->smf_n4_teid); @@ -329,17 +337,40 @@ uint8_t smf_s5c_handle_create_session_request( smf_bearer_remove_all(sess); /* Setup Bearer */ + bearer_count = 0; + for (i = 0; i < OGS_BEARER_PER_UE; i++) { if (req->bearer_contexts_to_be_created[i].presence == 0) break; + + bearer_count++; + + if (bearer_count > 1) { + ogs_error("Unexpected multiple Bearer Contexts in " + "Create Session Request"); + smf_bearer_remove_all(sess); + return OGS_GTP2_CAUSE_MANDATORY_IE_INCORRECT; + } + if (req->bearer_contexts_to_be_created[i].eps_bearer_id.presence == 0) { ogs_error("No EPS Bearer ID"); - break; + smf_bearer_remove_all(sess); + return OGS_GTP2_CAUSE_MANDATORY_IE_MISSING; } + if (smf_bearer_find_by_ebi(sess, + req->bearer_contexts_to_be_created[i].eps_bearer_id.u8)) { + ogs_error("Duplicate EPS Bearer ID [%u] in " + "Create Session Request", + req->bearer_contexts_to_be_created[i].eps_bearer_id.u8); + smf_bearer_remove_all(sess); + return OGS_GTP2_CAUSE_MANDATORY_IE_INCORRECT; + } + if (req->bearer_contexts_to_be_created[i]. bearer_level_qos.presence == 0) { ogs_error("No Bearer QoS"); - break; + smf_bearer_remove_all(sess); + return OGS_GTP2_CAUSE_MANDATORY_IE_MISSING; } decoded = ogs_gtp2_parse_bearer_qos(&bearer_qos, @@ -347,6 +378,7 @@ uint8_t smf_s5c_handle_create_session_request( if (GTP2_BEARER_QOS_LEN != decoded) { ogs_error("Invalid Bearer QoS IE in Create Session Request " "(decoded=%d, expected=%d)", decoded, GTP2_BEARER_QOS_LEN); + smf_bearer_remove_all(sess); return OGS_GTP2_CAUSE_MANDATORY_IE_INCORRECT; } @@ -363,8 +395,11 @@ uint8_t smf_s5c_handle_create_session_request( ogs_assert(sgw_s5u_teid); bearer->sgw_s5u_teid = be32toh(sgw_s5u_teid->teid); rv = ogs_gtp2_f_teid_to_ip(sgw_s5u_teid, &bearer->sgw_s5u_ip); - ogs_assert(rv == OGS_OK); - + if (rv != OGS_OK) { + ogs_error("Invalid SGW-S5U TEID"); + smf_bearer_remove_all(sess); + return OGS_GTP2_CAUSE_MANDATORY_IE_INCORRECT; + } break; case OGS_GTP2_RAT_TYPE_WLAN: sgw_s5u_teid = req->bearer_contexts_to_be_created[i]. @@ -372,7 +407,11 @@ uint8_t smf_s5c_handle_create_session_request( ogs_assert(sgw_s5u_teid); bearer->sgw_s5u_teid = be32toh(sgw_s5u_teid->teid); rv = ogs_gtp2_f_teid_to_ip(sgw_s5u_teid, &bearer->sgw_s5u_ip); - ogs_assert(rv == OGS_OK); + if (rv != OGS_OK) { + ogs_error("Invalid SGW-S5U TEID"); + smf_bearer_remove_all(sess); + return OGS_GTP2_CAUSE_MANDATORY_IE_INCORRECT; + } break; default: ogs_error("Unknown RAT Type [%d]", sess->gtp_rat_type); @@ -393,6 +432,12 @@ uint8_t smf_s5c_handle_create_session_request( } } + if (bearer_count == 0) { + ogs_error("No Bearer Context"); + smf_bearer_remove_all(sess); + return OGS_GTP2_CAUSE_MANDATORY_IE_MISSING; + } + /* Set AMBR if available */ if (req->aggregate_maximum_bit_rate.presence) { /*