[MERGED] osmo-iuh[master]: hnbap, rua, ranap decode: fix segfault on decode error

[Neels Hofmeyr]

Neels Hofmeyr has submitted this change and it was merged.

Change subject: hnbap,rua,ranap decode: fix segfault on decode error
......................................................................


hnbap,rua,ranap decode: fix segfault on decode error

Looking at hnbap_decode_hnbregisterrequesties(), I noticed a segfault if
decoding the HNB Register Request PDU fails, which is due to an unchecked
return value in code generated by asn1tostruct.py.

Add return value and NULL pointer checks and hence fix null dereference on
erratic PDUs across HNBAP, RUA and RANAP protocols. Similar checks exist in
other places, this one was simply missing.

Since the result of asn1tostruct.py is not committed, here is an example diff
of the resulting change, of which there are 128 instances in total:

@@ -304,7 +329,12 @@
     memset(hnbRegisterRequestIEs, 0, sizeof(HNBRegisterRequestIEs_t));
     HNBAP_DEBUG("Decoding message HNBRegisterRequestIEs (%s:%d)\n", __FILE__, __LINE__);

-    ANY_to_type_aper(any_p, &asn_DEF_HNBRegisterRequest, (void**)&hNBRegisterRequest_p);
+    tempDecoded = ANY_to_type_aper(any_p, &asn_DEF_HNBRegisterRequest, (void**)&hNBRegisterRequest_p);
+
+    if (tempDecoded < 0 || hNBRegisterRequest_p == NULL) {
+        HNBAP_DEBUG("Decoding of message HNBRegisterRequestIEs failed\n");
+        return -1;
+    }

     for (i = 0; i < hNBRegisterRequest_p->hnbRegisterRequest_ies.list.count; i++) {
         IE_t *ie_p;

Change-Id: I6cb9cc9a88d22f03befa43f0968a874476fa079d
---
M asn1/utils/asn1tostruct.py
1 file changed, 6 insertions(+), 1 deletion(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/asn1/utils/asn1tostruct.py b/asn1/utils/asn1tostruct.py
index 8364c27..5586e40 100755
--- a/asn1/utils/asn1tostruct.py
+++ b/asn1/utils/asn1tostruct.py
@@ -311,7 +311,12 @@
         f.write("    memset(%s, 0, sizeof(%s_t));\n" % (lowerFirstCamelWord(re.sub('-', '_', key)), prefix + re.sub('-', '_', key)))
 
     f.write("    %s_DEBUG(\"Decoding message %s (%%s:%%d)\\n\", __FILE__, __LINE__);\n\n" % (fileprefix.upper(), prefix + re.sub('-', '_', keyName)))
-    f.write("    ANY_to_type_aper(any_p, &asn_DEF_%s, (void**)&%s_p);\n\n" % (asn1cStruct, asn1cStructfirstlower))
+    f.write("    tempDecoded = ANY_to_type_aper(any_p, &asn_DEF_%s, (void**)&%s_p);\n\n" % (asn1cStruct, asn1cStructfirstlower))
+    f.write("    if (tempDecoded < 0 || %s_p == NULL) {\n" % (asn1cStructfirstlower))
+    f.write("        %s_DEBUG(\"Decoding of message %s failed\\n\");\n" % (fileprefix.upper(), prefix + re.sub('-', '_', keyName)))
+    f.write("        return -1;\n")
+    f.write("    }\n\n")
+
     f.write("    for (i = 0; i < %s_p->%slist.count; i++) {\n" % (asn1cStructfirstlower, iesaccess))
     f.write("        %sIE_t *ie_p;\n" % (prefix))
     f.write("        ie_p = %s_p->%slist.array[i];\n" % (asn1cStructfirstlower, iesaccess))

-- 
To view, visit https://gerrit.osmocom.org/5531
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I6cb9cc9a88d22f03befa43f0968a874476fa079d
Gerrit-PatchSet: 1
Gerrit-Project: osmo-iuh
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>