[SEC] Vulnerabilities have been resolved (open5gs#2945)

Reachable assertion in amf_ue_set_suci Location: src/amf/context.c:1968 ``` void amf_ue_set_suci(amf_ue_t *amf_ue, ogs_nas_5gs_mobile_identity_t *mobile_identity) { amf_ue_t *old_amf_ue = NULL; amf_sess_t *old_sess = NULL; char *suci = NULL; ogs_assert(amf_ue); ogs_assert(mobile_identity); suci = ogs_nas_5gs_suci_from_mobile_identity(mobile_identity); ogs_assert(suci); ``` Exploitable by: Base Station Severity: denial of service
pespin · Mar 5, 2024 · 322719f · 322719f
1 parent 199f4c7
commit 322719f
Show file tree

Hide file tree

Showing 6 changed files with 93 additions and 2 deletions.
diff --git a/lib/nas/5gs/conv.c b/lib/nas/5gs/conv.c
@@ -110,13 +110,15 @@ char *ogs_nas_5gs_suci_from_mobile_identity(
                 ogs_plmn_id_mcc(&plmn_id), ogs_plmn_id_mnc(&plmn_id));
         if (!suci) {
             ogs_error("ogs_mstrcatf() failed");
+            ogs_free(suci);
             return NULL;
         }
     } else {
         suci = ogs_mstrcatf(suci, "%03d-%03d-",
                 ogs_plmn_id_mcc(&plmn_id), ogs_plmn_id_mnc(&plmn_id));
         if (!suci) {
             ogs_error("ogs_mstrcatf() failed");
+            ogs_free(suci);
             return NULL;
         }
     }
@@ -152,9 +154,14 @@ char *ogs_nas_5gs_suci_from_mobile_identity(
     scheme_output =
         (uint8_t *)mobile_identity->buffer +
         OGS_NAS_5GS_MOBILE_IDENTITY_SUCI_MIN_SIZE;
+    if (mobile_identity->length < OGS_NAS_5GS_MOBILE_IDENTITY_SUCI_MIN_SIZE) {
+        ogs_error("The length of Mobile Identity(%d) is less then the min(%d)",
+            mobile_identity->length, OGS_NAS_5GS_MOBILE_IDENTITY_SUCI_MIN_SIZE);
+        ogs_free(suci);
+        return NULL;
+    }
     scheme_output_size =
         mobile_identity->length - OGS_NAS_5GS_MOBILE_IDENTITY_SUCI_MIN_SIZE;
-    ogs_assert(scheme_output_size);
     scheme_output_string_or_bcd = ogs_calloc(1, scheme_output_size*2+1);
     ogs_assert(scheme_output_string_or_bcd);
 

diff --git a/src/amf/gmm-handler.c b/src/amf/gmm-handler.c
@@ -130,6 +130,12 @@ ogs_nas_5gmm_cause_t gmm_handle_registration_request(amf_ue_t *amf_ue,
         return OGS_5GMM_CAUSE_SEMANTICALLY_INCORRECT_MESSAGE;
     }
 
+    if (mobile_identity->length < OGS_NAS_5GS_MOBILE_IDENTITY_SUCI_MIN_SIZE) {
+        ogs_error("The length of Mobile Identity(%d) is less then the min(%d)",
+            mobile_identity->length, OGS_NAS_5GS_MOBILE_IDENTITY_SUCI_MIN_SIZE);
+        return OGS_5GMM_CAUSE_SEMANTICALLY_INCORRECT_MESSAGE;
+    }
+
     mobile_identity_header =
             (ogs_nas_5gs_mobile_identity_header_t *)mobile_identity->buffer;
 
@@ -886,6 +892,12 @@ ogs_nas_5gmm_cause_t gmm_handle_identity_response(amf_ue_t *amf_ue,
         return OGS_5GMM_CAUSE_SEMANTICALLY_INCORRECT_MESSAGE;
     }
 
+    if (mobile_identity->length < OGS_NAS_5GS_MOBILE_IDENTITY_SUCI_MIN_SIZE) {
+        ogs_error("The length of Mobile Identity(%d) is less then the min(%d)",
+            mobile_identity->length, OGS_NAS_5GS_MOBILE_IDENTITY_SUCI_MIN_SIZE);
+        return OGS_5GMM_CAUSE_SEMANTICALLY_INCORRECT_MESSAGE;
+    }
+
     mobile_identity_header =
             (ogs_nas_5gs_mobile_identity_header_t *)mobile_identity->buffer;
 

diff --git a/src/amf/gmm-sm.c b/src/amf/gmm-sm.c
@@ -1365,7 +1365,17 @@ static void common_register_state(ogs_fsm_t *s, amf_event_t *e,
                 ogs_error("gmm_handle_identity_response() "
                             "failed [%d] in type [%d]",
                             gmm_cause, amf_ue->nas.message_type);
-                r = nas_5gs_send_gmm_reject(ran_ue, amf_ue, gmm_cause);
+                if (amf_ue->nas.message_type ==
+                        OGS_NAS_5GS_REGISTRATION_REQUEST ||
+                    amf_ue->nas.message_type ==
+                        OGS_NAS_5GS_SERVICE_REQUEST)
+                    r = nas_5gs_send_gmm_reject(ran_ue, amf_ue, gmm_cause);
+                else
+                    r = ngap_send_error_indication2(
+                            ran_ue,
+                            NGAP_Cause_PR_protocol,
+                            NGAP_CauseProtocol_semantic_error);
+
                 ogs_expect(r == OGS_OK);
                 ogs_assert(r != OGS_ERROR);
                 OGS_FSM_TRAN(s, gmm_state_exception);

diff --git a/tests/common/ngap-build.c b/tests/common/ngap-build.c
@@ -2661,3 +2661,30 @@ ogs_pkbuf_t *test_ngap_build_amf_configuration_ack(int i)
 
     return pkbuf;
 }
+
+ogs_pkbuf_t *test_ngap_build_malformed_initial_ue_message(int i)
+{
+    ogs_pkbuf_t *pkbuf = NULL;
+    const char *payload[TEST_NGAP_MAX_MESSAGE] = {
+        "000f007300000700 5500034002000026 001d1c0602940a5f 7f5f7e105c000209"
+        "00007fff00000000 004c4c585f4e5f00 79000f405f7a8a1f 58755ff001940078"
+        "954e005a40012800 0340025fc0007040 010000ab4021205f 5f5f5f4f3d7fff10"
+        "de5f5f765f000000 0000000000000000 00000000000000"
+        "",
+        "",
+
+    };
+    uint16_t len[TEST_NGAP_MAX_MESSAGE] = {
+        119,
+        0,
+        0,
+    };
+    char hexbuf[OGS_HUGE_LEN];
+
+    pkbuf = ogs_pkbuf_alloc(NULL, OGS_MAX_SDU_LEN);
+    ogs_assert(pkbuf);
+    ogs_pkbuf_put_data(pkbuf,
+        ogs_hex_from_string(payload[i], hexbuf, sizeof(hexbuf)), len[i]);
+
+    return pkbuf;
+}
diff --git a/tests/common/ngap-build.h b/tests/common/ngap-build.h
@@ -80,6 +80,7 @@ ogs_pkbuf_t *testngap_build_handover_cancel(test_ue_t *test_ue,
         NGAP_Cause_PR group, long cause);
 
 ogs_pkbuf_t *test_ngap_build_amf_configuration_ack(int i);
+ogs_pkbuf_t *test_ngap_build_malformed_initial_ue_message(int i);
 
 #ifdef __cplusplus
 }

diff --git a/tests/registration/crash-test.c b/tests/registration/crash-test.c
@@ -1401,6 +1401,39 @@ static void test4_issues2842_func(abts_case *tc, void *data)
     test_ue_remove(test_ue);
 }
 
+static void test5_func(abts_case *tc, void *data)
+{
+    int rv;
+    ogs_socknode_t *ngap;
+    ogs_pkbuf_t *sendbuf;
+    ogs_pkbuf_t *recvbuf;
+    ogs_ngap_message_t message;
+
+    ngap = testngap_client(AF_INET);
+    ABTS_PTR_NOTNULL(tc, ngap);
+
+    sendbuf = testngap_build_ng_setup_request(0x4000, 22);
+    ABTS_PTR_NOTNULL(tc, sendbuf);
+    rv = testgnb_ngap_send(ngap, sendbuf);
+    ABTS_INT_EQUAL(tc, OGS_OK, rv);
+
+    recvbuf = testgnb_ngap_read(ngap);
+    ABTS_PTR_NOTNULL(tc, recvbuf);
+    ogs_pkbuf_free(recvbuf);
+
+    sendbuf = test_ngap_build_malformed_initial_ue_message(0);
+    ABTS_PTR_NOTNULL(tc, sendbuf);
+
+    rv = testgnb_ngap_send(ngap, sendbuf);
+    ABTS_INT_EQUAL(tc, OGS_OK, rv);
+
+    recvbuf = testgnb_ngap_read(ngap);
+    ABTS_PTR_NOTNULL(tc, recvbuf);
+    ogs_pkbuf_free(recvbuf);
+
+    testgnb_ngap_close(ngap);
+}
+
 abts_suite *test_crash(abts_suite *suite)
 {
     suite = ADD_SUITE(suite)
@@ -1409,6 +1442,7 @@ abts_suite *test_crash(abts_suite *suite)
     abts_run_test(suite, test2_func, NULL);
     abts_run_test(suite, test3_func, NULL);
     abts_run_test(suite, test4_issues2842_func, NULL);
+    abts_run_test(suite, test5_func, NULL);
 
     return suite;
 }