From f7ec6ea2edfb4451fa7b2dbdb0f10a654ab3cc64 Mon Sep 17 00:00:00 2001 From: Sukchan Lee Date: Sat, 14 Mar 2026 08:43:59 +0900 Subject: [PATCH] gtp: harden parsers against malformed IE lengths and remove assert-based crashes This patch improves robustness of several GTPv1/v2 parsing paths by adding explicit length validation and replacing assert-based checks on network-controlled data with graceful error handling. Changes include: - GTPv1 MM Context parser: Add bounds checks for xres_len, autn_len and num_vectors to prevent stack overflows when decoding authentication quintuplets. - SMF Gn handler: Validate IMEI(SV) IE length before memcpy to prevent heap overflow in smf_ue->imeisv. - SMF Gn handler: Validate Common Flags IE length before dereferencing to avoid out-of-bounds reads when malformed IE is received. - GTPv1 ULI parser: Replace ogs_assert-based length checks with proper validation and error return to prevent abort() on truncated User Location Information IE. - SMF fd-path: Replace assertions on ULI payload presence with runtime checks to avoid process termination on malformed input. These changes ensure malformed or truncated network messages are handled gracefully instead of triggering process aborts. --- lib/gtp/v1/types.c | 35 +++++++++++++++++++++++++++++++---- src/smf/fd-path.c | 7 +++++-- src/smf/gn-handler.c | 12 ++++++++++-- 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/lib/gtp/v1/types.c b/lib/gtp/v1/types.c index ce1982199..3c018a0ee 100644 --- a/lib/gtp/v1/types.c +++ b/lib/gtp/v1/types.c @@ -31,12 +31,21 @@ int16_t ogs_gtp1_parse_uli(ogs_gtp1_uli_t *uli, ogs_tlv_octet_t *octet) memset(uli, 0, sizeof(ogs_gtp1_uli_t)); + if (octet->len < 1) { + ogs_error("ULI IE too short [%u]", octet->len); + return 0; + } + uli->geo_loc_type = source->geo_loc_type; size++; switch (uli->geo_loc_type) { case OGS_GTP1_GEO_LOC_TYPE_CGI: - ogs_assert(size + sizeof(uli->cgi) <= octet->len); + if (size + (int)sizeof(uli->cgi) > octet->len) { + ogs_error("ULI CGI too short [%d+%zu > %u]", + size, sizeof(uli->cgi), octet->len); + return 0; + } memcpy(&uli->cgi, (unsigned char *)octet->data + size, sizeof(uli->cgi)); uli->cgi.lac = be16toh(uli->cgi.lac); @@ -44,7 +53,11 @@ int16_t ogs_gtp1_parse_uli(ogs_gtp1_uli_t *uli, ogs_tlv_octet_t *octet) size += sizeof(uli->cgi); break; case OGS_GTP1_GEO_LOC_TYPE_SAI: - ogs_assert(size + sizeof(uli->sai) <= octet->len); + if (size + (int)sizeof(uli->sai) > octet->len) { + ogs_error("ULI SAI too short [%d+%zu > %u]", + size, sizeof(uli->sai), octet->len); + return 0; + } memcpy(&uli->sai, (unsigned char *)octet->data + size, sizeof(uli->sai)); uli->sai.lac = be16toh(uli->sai.lac); @@ -52,7 +65,11 @@ int16_t ogs_gtp1_parse_uli(ogs_gtp1_uli_t *uli, ogs_tlv_octet_t *octet) size += sizeof(uli->sai); break; case OGS_GTP1_GEO_LOC_TYPE_RAI: - ogs_assert(size + sizeof(uli->rai) <= octet->len); + if (size + (int)sizeof(uli->rai) > octet->len) { + ogs_error("ULI RAI too short [%d+%zu > %u]", + size, sizeof(uli->rai), octet->len); + return 0; + } memcpy(&uli->rai, (unsigned char *)octet->data + size, sizeof(uli->rai)); uli->rai.lac = be16toh(uli->rai.lac); @@ -65,7 +82,11 @@ int16_t ogs_gtp1_parse_uli(ogs_gtp1_uli_t *uli, ogs_tlv_octet_t *octet) return 0; } - ogs_assert(size == octet->len); + if (size != octet->len) { + ogs_error("Mismatch IE Length[%d] != Decoded[%d]", octet->len, size); + return 0; + } + return size; } int16_t ogs_gtp1_build_uli( @@ -414,6 +435,8 @@ static int decode_quintuple(ogs_gtp1_auth_quintuplet_t *decoded, uint8_t *data, CHECK_SPACE_ERR(1); decoded->xres_len = *ptr++; + if (decoded->xres_len > OGS_MAX_RES_LEN) + return OGS_ERROR; CHECK_SPACE_ERR(decoded->xres_len); memcpy(&decoded->xres[0], ptr, decoded->xres_len); ptr += decoded->xres_len; @@ -428,6 +451,8 @@ static int decode_quintuple(ogs_gtp1_auth_quintuplet_t *decoded, uint8_t *data, CHECK_SPACE_ERR(1); decoded->autn_len = *ptr++; + if (decoded->autn_len > OGS_AUTN_LEN) + return OGS_ERROR; CHECK_SPACE_ERR(decoded->autn_len); memcpy(&decoded->autn[0], ptr, decoded->autn_len); ptr += decoded->autn_len; @@ -473,6 +498,8 @@ int ogs_gtp1_parse_mm_context( decoded->ksi = *ptr & 0x07; ptr++; decoded->num_vectors = (*ptr >> 3) & 0x07; + if (decoded->num_vectors > OGS_ARRAY_SIZE(decoded->auth_quintuplets)) + return OGS_ERROR; decoded->used_cipher = *ptr & 0x07; ptr++; diff --git a/src/smf/fd-path.c b/src/smf/fd-path.c index 0186f840f..31f1eb29c 100644 --- a/src/smf/fd-path.c +++ b/src/smf/fd-path.c @@ -105,8 +105,11 @@ void smf_fd_msg_avp_add_3gpp_uli(smf_sess_t *sess, struct msg *req) return; } - ogs_assert(sess->gtp.user_location_information.data); - ogs_assert(sess->gtp.user_location_information.len); + if (!sess->gtp.user_location_information.data || + !sess->gtp.user_location_information.len) { + ogs_error("Missing User Location Information(ULI) payload"); + return; + } memcpy(&uli_buf, sess->gtp.user_location_information.data, sess->gtp.user_location_information.len); diff --git a/src/smf/gn-handler.c b/src/smf/gn-handler.c index b27c77e74..64b96ddd6 100644 --- a/src/smf/gn-handler.c +++ b/src/smf/gn-handler.c @@ -186,7 +186,8 @@ uint8_t smf_gn_handle_create_pdp_context_request( } /* Common Flags 7.7.48 */ - if (req->common_flags.presence) { + if (req->common_flags.presence && + req->common_flags.len >= sizeof(ogs_gtp1_common_flags_t)) { sess->gtp.v1.common_flags = *(ogs_gtp1_common_flags_t*)req->common_flags.data; } @@ -241,6 +242,12 @@ uint8_t smf_gn_handle_create_pdp_context_request( /* Set IMEI(SV) */ if (req->imei.presence && req->imei.len > 0) { + if (req->imei.len > sizeof(smf_ue->imeisv)) { + ogs_error("IMEI(SV) wrong size %u > %zu", + req->imei.len, sizeof(smf_ue->imeisv)); + return OGS_GTP1_CAUSE_MANDATORY_IE_INCORRECT; + } + smf_ue->imeisv_len = req->imei.len; memcpy(smf_ue->imeisv, (uint8_t*)req->imei.data, smf_ue->imeisv_len); @@ -424,7 +431,8 @@ void smf_gn_handle_update_pdp_context_request( } /* Common Flags 7.7.48 */ - if (req->common_flags.presence) { + if (req->common_flags.presence && + req->common_flags.len >= sizeof(ogs_gtp1_common_flags_t)) { sess->gtp.v1.common_flags = *(ogs_gtp1_common_flags_t*)req->common_flags.data; } else { /* Reset it to overwrite what was received during CreatePDPCtxReq time */