fix(mme): Enforce stricter input validation for S1AP/NGAP/NAS payloads

Signed-off-by: Nathaniel <me@nathanielbennett.com>
magma · Jun 17, 2024 · d140ee3 · d140ee3
1 parent 50e0d51
commit d140ee3
Show file tree

Hide file tree

Showing 10 changed files with 104 additions and 18 deletions.
diff --git a/lte/gateway/c/core/oai/lib/3gpp/3gpp_24.008_mm_ies.c b/lte/gateway/c/core/oai/lib/3gpp/3gpp_24.008_mm_ies.c
@@ -543,6 +543,7 @@ int decode_emergency_number_list_ie(
     emergency_number_list_t* emergencynumberlist, const bool iei_present,
     uint8_t* buffer, const uint32_t len) {
   int decoded = 0;
+  int signed_len = len;
   uint8_t ielen = 0;
   emergency_number_list_t* e = emergencynumberlist;
 
@@ -564,6 +565,8 @@ int decode_emergency_number_list_ie(
       buffer + decoded, EMERGENCY_NUMBER_MAX_DIGITS, len - decoded);
 
   e->lengthofemergencynumberinformation = *(buffer + decoded);
+  CHECK_LENGTH_DECODER(signed_len - decoded,
+                       e->lengthofemergencynumberinformation);
 
   decoded++;
   emergencynumberlist->emergencyservicecategoryvalue =
@@ -577,7 +580,9 @@ int decode_emergency_number_list_ie(
        i < EMERGENCY_NUMBER_MAX_DIGITS; i++) {
     e->number_digit[i] = 0xFF;
   }
-  Fatal("TODO emergency_number_list_t->next");
+
+  // implement emergency_number_list_t->next
+  return TLV_PROTOCOL_NOT_SUPPORTED;
 
   return decoded;
 }
@@ -590,7 +595,9 @@ int encode_emergency_number_list_ie(
   uint32_t encoded = 0;
   emergency_number_list_t* e = emergencynumberlist;
 
-  Fatal("TODO Implement encode_emergency_number_list_ie");
+  // implement encode_emergency_number_list_ie
+  return TLV_PROTOCOL_NOT_SUPPORTED;
+
   if (iei_present) {
     CHECK_PDU_POINTER_AND_LENGTH_ENCODER(
         buffer, EMERGENCY_NUMBER_LIST_IE_MIN_LENGTH, len);

diff --git a/lte/gateway/c/core/oai/lib/3gpp/3gpp_24.008_sm_ies.c b/lte/gateway/c/core/oai/lib/3gpp/3gpp_24.008_sm_ies.c
@@ -56,8 +56,9 @@
 int decode_access_point_name_ie(access_point_name_t* access_point_name,
                                 bool is_ie_present, uint8_t* buffer,
                                 const uint32_t len) {
+  int signed_len = len;
   int decoded = 0;
-  uint8_t ielen = 0;
+  int ielen = 0;
 
   *access_point_name = NULL;
 
@@ -73,13 +74,16 @@ int decode_access_point_name_ie(access_point_name_t* access_point_name,
 
   ielen = *(buffer + decoded);
   decoded++;
-  CHECK_LENGTH_DECODER(len - decoded, ielen);
+  CHECK_LENGTH_DECODER(signed_len - decoded, ielen);
 
   if (1 <= ielen) {
     int length_apn = *(buffer + decoded);
     decoded++;
+    CHECK_LENGTH_DECODER(ielen, length_apn + 1);
+    // ^ ielen <= (signed_len - decoded), so that is implicitly checked here too
     *access_point_name = blk2bstr((void*)(buffer + decoded), length_apn);
     decoded += length_apn;
+
     ielen = ielen - 1 - length_apn;
     while (1 <= ielen) {
       bconchar(*access_point_name, '.');
@@ -89,9 +93,10 @@ int decode_access_point_name_ie(access_point_name_t* access_point_name,
 
       // apn terminated by '.' ?
       if (length_apn > 0) {
-        AssertFatal(ielen >= length_apn,
-                    "Mismatch in lengths remaining ielen %d apn length %d",
-                    ielen, length_apn);
+        if (ielen < length_apn) {
+          // Mismatch in lengths remaining between IE length and APN length
+          return TLV_OCTET_STRING_TOO_LONG_FOR_IEI;
+        }
         bcatblk(*access_point_name, (void*)(buffer + decoded), length_apn);
         decoded += length_apn;
         ielen = ielen - length_apn;
@@ -236,6 +241,10 @@ int decode_protocol_configuration_options(
   protocolconfigurationoptions->num_protocol_or_container_id = 0;
 
   while (3 <= ((int32_t)len - (int32_t)decoded)) {
+    if (protocolconfigurationoptions->num_protocol_or_container_id >=
+        PCO_UNSPEC_MAXIMUM_PROTOCOL_ID_OR_CONTAINER_ID) {
+      return TLV_UNEXPECTED_IEI;  // Maximum 30 options
+    }
     DECODE_U16(
         buffer + decoded,
         protocolconfigurationoptions
@@ -492,14 +501,14 @@ int encode_quality_of_service_ie(quality_of_service_t* qualityofservice,
 //------------------------------------------------------------------------------
 int encode_linked_ti_ie(linked_ti_t* linkedti, const bool iei_present,
                         uint8_t* buffer, const uint32_t len) {
-  Fatal("TODO Implement encode_linked_ti_ie");
-  return -1;
+  // Implement encode_linked_ti_ie
+  return TLV_PROTOCOL_NOT_SUPPORTED;
 }
 
 int decode_linked_ti_ie(linked_ti_t* linkedti, const bool iei_present,
                         uint8_t* buffer, const uint32_t len) {
-  Fatal("TODO Implement decode_linked_ti_ie");
-  return -1;
+  // Implement decode_linked_ti_ie
+  return TLV_PROTOCOL_NOT_SUPPORTED;
 }
 
 //------------------------------------------------------------------------------
@@ -618,6 +627,11 @@ static int decode_traffic_flow_template_packet_filter(
     packet_filter_t* packetfilter, const uint8_t* const buffer,
     const uint32_t len) {
   int decoded = 0, j;
+  int signed_len = len;
+
+  if (len < 3) {
+    return TLV_BUFFER_TOO_SHORT;
+  }
 
   if (len - decoded <= 0) {
     /*
@@ -655,6 +669,10 @@ static int decode_traffic_flow_template_packet_filter(
    */
   int pkfstart = decoded;
 
+  if (len < pkfstart + pkflen) {
+    return TLV_BUFFER_TOO_SHORT;
+  }
+
   while (decoded - pkfstart < pkflen) {
     /*
      * Packet filter component type identifier
@@ -803,7 +821,7 @@ static int decode_traffic_flow_template_packet_filter(
     }
   }
 
-  if (len - decoded < 0) {
+  if (signed_len - decoded < 0) {
     /*
      * Decoded more than remaining space in decoding buffer
      */

diff --git a/lte/gateway/c/core/oai/tasks/amf/amf_as.cpp b/lte/gateway/c/core/oai/tasks/amf/amf_as.cpp
@@ -149,7 +149,8 @@ static status_code_e amf_as_establish_req(amf_as_establish_t* msg,
     }
   }
 
-  if ((msg->nas_msg->data[1] != 0x0) && (msg->nas_msg->data[9] == 0x5c)) {
+  if ((blength(msg->nas_msg) > 9) && (msg->nas_msg->data[1] != 0x0) &&
+      (msg->nas_msg->data[9] == 0x5c)) {
     for (int i = 0, j = 7; j < blength(msg->nas_msg); i++, j++) {
       msg->nas_msg->data[i] = msg->nas_msg->data[j];
     }

diff --git a/lte/gateway/c/core/oai/tasks/nas/emm/sap/emm_as.cpp b/lte/gateway/c/core/oai/tasks/nas/emm/sap/emm_as.cpp
@@ -807,6 +807,16 @@ static status_code_e emm_as_establish_req(emm_as_establish_t* msg,
     }
   }
 
+  if (nas_msg.plain.emm.header.protocol_discriminator !=
+      EPS_MOBILITY_MANAGEMENT_MESSAGE) {
+    // The NAS message had external EMM header and internal internal
+    // plaintext/encrypted ESM header--discard
+    OAILOG_ERROR(LOG_NAS_EMM,
+                 "EMMAS-SAP - Malformed Packet - Inconsistent outer EMM header "
+                 "with inner ESM header");
+  }
+
+
   /*
    * Process initial NAS message
    */

diff --git a/lte/gateway/c/core/oai/tasks/nas/ies/EsmMessageContainer.cpp b/lte/gateway/c/core/oai/tasks/nas/ies/EsmMessageContainer.cpp
@@ -46,7 +46,7 @@ int decode_esm_message_container(EsmMessageContainer* esmmessagecontainer,
   }
 
   DECODE_LENGTH_U16(buffer + decoded, ielen, decoded);
-  CHECK_LENGTH_DECODER(len - decoded, ielen);
+  CHECK_LENGTH_DECODER(static_cast<int>(len) - decoded, ielen);
 
   if ((decode_result = decode_bstring(esmmessagecontainer, ielen,
                                       buffer + decoded, len - decoded)) < 0) {

diff --git a/lte/gateway/c/core/oai/tasks/nas/ies/PdnAddress.cpp b/lte/gateway/c/core/oai/tasks/nas/ies/PdnAddress.cpp
@@ -31,18 +31,22 @@ extern "C" {
 //------------------------------------------------------------------------------
 int decode_pdn_address(PdnAddress* pdnaddress, uint8_t iei, uint8_t* buffer,
                        uint32_t len) {
+  int signed_len = len;
   int decoded = 0;
   uint8_t ielen = 0;
   int decode_result;
 
   if (iei > 0) {
+    CHECK_LENGTH_DECODER(signed_len - decoded, 1);
     CHECK_IEI_DECODER(iei, *buffer);
     decoded++;
   }
 
+  CHECK_LENGTH_DECODER(signed_len - decoded, 2);
+
   ielen = *(buffer + decoded);
   decoded++;
-  CHECK_LENGTH_DECODER(len - decoded, ielen);
+  CHECK_LENGTH_DECODER(signed_len - decoded, ielen);
   pdnaddress->pdntypevalue = *(buffer + decoded) & 0x7;
   decoded++;
 

diff --git a/lte/gateway/c/core/oai/tasks/ngap/ngap_amf_handlers.c b/lte/gateway/c/core/oai/tasks/ngap/ngap_amf_handlers.c
@@ -393,6 +393,9 @@ status_code_e ngap_amf_handle_ng_setup_request(ngap_state_t* state,
 
   NGAP_FIND_PROTOCOLIE_BY_ID(Ngap_NGSetupRequestIEs_t, ie, container,
                              Ngap_ProtocolIE_ID_id_GlobalRANNodeID, true);
+  if (!ie) {
+    OAILOG_FUNC_RETURN(LOG_NGAP, RETURNerror);
+  }
   if (ie->value.choice.GlobalRANNodeID.choice.globalGNB_ID.gNB_ID.present ==
       Ngap_GNB_ID_PR_gNB_ID) {
     gnb_id = BIT_STRING_to_uint32(&ie->value.choice.GlobalRANNodeID.choice
@@ -412,6 +415,9 @@ status_code_e ngap_amf_handle_ng_setup_request(ngap_state_t* state,
   NGAP_FIND_PROTOCOLIE_BY_ID(Ngap_NGSetupRequestIEs_t, ie_supported_tas,
                              container, Ngap_ProtocolIE_ID_id_SupportedTAList,
                              true);
+  if (!ie_supported_tas) {
+    OAILOG_FUNC_RETURN(LOG_NGAP, RETURNerror);
+  }
 
   ta_ret = ngap_amf_compare_ta_lists(
       &ie_supported_tas->value.choice.SupportedTAList);
@@ -498,6 +504,9 @@ status_code_e ngap_amf_handle_ng_setup_request(ngap_state_t* state,
   NGAP_FIND_PROTOCOLIE_BY_ID(Ngap_NGSetupRequestIEs_t, ie_default_paging_drx,
                              container, Ngap_ProtocolIE_ID_id_DefaultPagingDRX,
                              true);
+  if (!ie_default_paging_drx) {
+    OAILOG_FUNC_RETURN(LOG_NGAP, RETURNerror);
+  }
 
   gnb_association->default_paging_drx =
       ie_default_paging_drx->value.choice.PagingDRX;
@@ -1690,6 +1699,9 @@ status_code_e ngap_amf_handle_pduSession_release_response(
   NGAP_FIND_PROTOCOLIE_BY_ID(Ngap_PDUSessionResourceReleaseResponseIEs_t, ie,
                              container, Ngap_ProtocolIE_ID_id_AMF_UE_NGAP_ID,
                              true);
+  if (!ie) {
+    OAILOG_FUNC_RETURN(LOG_NGAP, RETURNerror);
+  }
   asn_INTEGER2ulong(&ie->value.choice.AMF_UE_NGAP_ID,
                     (uint64_t*)&amf_ue_ngap_id);
 
@@ -1705,6 +1717,9 @@ status_code_e ngap_amf_handle_pduSession_release_response(
   NGAP_FIND_PROTOCOLIE_BY_ID(Ngap_PDUSessionResourceReleaseResponseIEs_t, ie,
                              container, Ngap_ProtocolIE_ID_id_RAN_UE_NGAP_ID,
                              true);
+  if (!ie) {
+    OAILOG_FUNC_RETURN(LOG_NGAP, RETURNerror);
+  }
   // gNB UE NGAP ID is limited to 24 bits
   gnb_ue_ngap_id = (gnb_ue_ngap_id_t)(ie->value.choice.RAN_UE_NGAP_ID);
 
@@ -2289,6 +2304,9 @@ status_code_e ngap_amf_handle_gnb_reset(ngap_state_t* state,
 
   NGAP_FIND_PROTOCOLIE_BY_ID(Ngap_NGResetIEs_t, ie, container,
                              Ngap_ProtocolIE_ID_id_ResetType, true);
+  if (!ie) {
+    OAILOG_FUNC_RETURN(LOG_NGAP, RETURNerror);
+  }
 
   Ngap_ResetType_t* resetType = &ie->value.choice.ResetType;
 

diff --git a/lte/gateway/c/core/oai/tasks/ngap/ngap_amf_nas_procedures.c b/lte/gateway/c/core/oai/tasks/ngap/ngap_amf_nas_procedures.c
@@ -366,19 +366,32 @@ status_code_e ngap_amf_handle_nas_non_delivery(ngap_state_t* state,
   }
   NGAP_FIND_PROTOCOLIE_BY_ID(Ngap_NASNonDeliveryIndication_IEs_t, ie, container,
                              Ngap_ProtocolIE_ID_id_AMF_UE_NGAP_ID, true);
+  if (!ie) {
+    OAILOG_FUNC_RETURN(LOG_NGAP, RETURNerror);
+  }
 
   asn_INTEGER2ulong(&ie->value.choice.AMF_UE_NGAP_ID,
                     (unsigned long*)&amf_ue_ngap_id);
 
   NGAP_FIND_PROTOCOLIE_BY_ID(Ngap_NASNonDeliveryIndication_IEs_t, ie, container,
                              Ngap_ProtocolIE_ID_id_RAN_UE_NGAP_ID, true);
+  if (!ie) {
+    OAILOG_FUNC_RETURN(LOG_NGAP, RETURNerror);
+  }
   gnb_ue_ngap_id = ie->value.choice.RAN_UE_NGAP_ID;
 
   NGAP_FIND_PROTOCOLIE_BY_ID(Ngap_NASNonDeliveryIndication_IEs_t, ie_nas_pdu,
                              container, Ngap_ProtocolIE_ID_id_NAS_PDU, true);
+  if (!ie_nas_pdu) {
+    OAILOG_FUNC_RETURN(LOG_NGAP, RETURNerror);
+  }
 
   NGAP_FIND_PROTOCOLIE_BY_ID(Ngap_NASNonDeliveryIndication_IEs_t, ie, container,
                              Ngap_ProtocolIE_ID_id_Cause, true);
+  if (!ie) {
+    OAILOG_FUNC_RETURN(LOG_NGAP, RETURNerror);
+  }
+
   OAILOG_NOTICE(LOG_NGAP,
                 "Received NGAP NAS_NON_DELIVERY_INDICATION message "
                 "AMF_UE_NGAP_ID " AMF_UE_NGAP_ID_FMT
@@ -407,8 +420,8 @@ status_code_e ngap_amf_handle_nas_non_delivery(ngap_state_t* state,
 
   // TODO: forward NAS PDU to NAS
   ngap_amf_itti_nas_non_delivery_ind(
-      amf_ue_ngap_id, ie->value.choice.NAS_PDU.buf,
-      ie->value.choice.NAS_PDU.size, &ie->value.choice.Cause, imsi64);
+      amf_ue_ngap_id, ie_nas_pdu->value.choice.NAS_PDU.buf,
+      ie_nas_pdu->value.choice.NAS_PDU.size, &ie->value.choice.Cause, imsi64);
   OAILOG_FUNC_RETURN(LOG_NGAP, RETURNok);
 }
 

diff --git a/lte/gateway/c/core/oai/tasks/ngap/ngap_common.h b/lte/gateway/c/core/oai/tasks/ngap/ngap_common.h
@@ -728,7 +728,6 @@ struct ngap_message_s;
             "NGAP_FIND_PROTOCOLIE_BY_ID: %s %d: Optional ie is NULL\n",       \
             __FILE__, __LINE__);                                              \
     }                                                                         \
-    if (mandatory) DevAssert(ie != NULL);                                     \
   } while (0)
 
 /** \brief Function callback prototype.

diff --git a/lte/gateway/c/core/oai/tasks/s1ap/s1ap_mme_handlers.cpp b/lte/gateway/c/core/oai/tasks/s1ap/s1ap_mme_handlers.cpp
@@ -4669,11 +4669,19 @@ status_code_e s1ap_mme_handle_erab_modification_indication(
   S1AP_FIND_PROTOCOLIE_BY_ID(S1ap_E_RABModificationIndicationIEs_t, ie,
                              container, S1ap_ProtocolIE_ID_id_MME_UE_S1AP_ID,
                              true);
+  if (!ie) {
+    OAILOG_ERROR(LOG_S1AP, "MME_UE_S1AP_ID Type S1AP ProtocolIE ID Missing\n");
+    OAILOG_FUNC_RETURN(LOG_S1AP, RETURNerror);
+  }
   mme_ue_s1ap_id = ie->value.choice.MME_UE_S1AP_ID;
 
   S1AP_FIND_PROTOCOLIE_BY_ID(S1ap_E_RABModificationIndicationIEs_t, ie,
                              container, S1ap_ProtocolIE_ID_id_eNB_UE_S1AP_ID,
                              true);
+  if (!ie) {
+    OAILOG_ERROR(LOG_S1AP, "eNB_UE_S1AP_ID Type S1AP ProtocolIE ID Missing\n");
+    OAILOG_FUNC_RETURN(LOG_S1AP, RETURNerror);
+  }
   // eNB UE S1AP ID is limited to 24 bits
   enb_ue_s1ap_id =
       (enb_ue_s1ap_id_t)(ie->value.choice.ENB_UE_S1AP_ID & ENB_UE_S1AP_ID_MASK);
@@ -5292,6 +5300,10 @@ status_code_e s1ap_mme_handle_erab_rel_response(oai::S1apState* state,
 
   S1AP_FIND_PROTOCOLIE_BY_ID(S1ap_E_RABReleaseResponseIEs_t, ie, container,
                              S1ap_ProtocolIE_ID_id_MME_UE_S1AP_ID, true);
+  if (!ie) {
+    OAILOG_ERROR(LOG_S1AP, "Missing MME_UE_S1AP_ID IE\n");
+    OAILOG_FUNC_RETURN(LOG_S1AP, RETURNerror);
+  }
   mme_ue_s1ap_id = ie->value.choice.MME_UE_S1AP_ID;
 
   if ((ie) && (ue_ref_p = s1ap_state_get_ue_mmeid((uint32_t)mme_ue_s1ap_id)) ==
@@ -5305,6 +5317,10 @@ status_code_e s1ap_mme_handle_erab_rel_response(oai::S1apState* state,
 
   S1AP_FIND_PROTOCOLIE_BY_ID(S1ap_E_RABReleaseResponseIEs_t, ie, container,
                              S1ap_ProtocolIE_ID_id_eNB_UE_S1AP_ID, true);
+  if (!ie) {
+    OAILOG_ERROR(LOG_S1AP, "Missing eNB_UE_S1AP_ID IE\n");
+    OAILOG_FUNC_RETURN(LOG_S1AP, RETURNerror);
+  }
   // eNB UE S1AP ID is limited to 24 bits
   enb_ue_s1ap_id =
       (enb_ue_s1ap_id_t)(ie->value.choice.ENB_UE_S1AP_ID & ENB_UE_S1AP_ID_MASK);