ooh323c/ooq931.c: Ensure ooQ931Decode doesn't run out-of-bounds.

Several bounds checks have been edded to ooQ931Decode to prevent it from
running past the end of the data buffer when parsing information elements.

Resolves: #GHSA-746q-794h-cc7f

diff --git a/addons/ooh323c/src/ooq931.c b/addons/ooh323c/src/ooq931.c
index fe8b06ebee5081554fea1d6e54bdfbf5a44948bc..bdcbae2993e5d493f56c66cb51d1b0ae1360790b 100644 (file)
--- a/addons/ooh323c/src/ooq931.c
+++ b/addons/ooh323c/src/ooq931.c
@@ -89,12 +89,21 @@ EXTERN int ooQ931Decode
    while (offset < length) {
       Q931InformationElement *ie;
       int ieOff = offset;
-      /* Get field discriminator */
+      /*
+       * Get field discriminator and advance offset to what should be
+       * the first (or only) byte of the IE length.
+       */
       int discriminator = data[offset++];
 
-      /* For discriminator with high bit set there is no data */
-      if ((discriminator & 0x80) == 0) {
-         int len = data[offset++], alen;
+      /*
+       * For discriminator with high bit == 0 there must be data.  The
+       * data length could be in the next 8 or 16 bits so there must be
+       * at least 1 byte after the discriminator.
+       *
+       * Make sure offset is still in-bounds.
+       */
+      if ((discriminator & 0x80) == 0 && offset < length) {
+         int len = data[offset], alen;
 
          if (discriminator == Q931UserUserIE) {
             /* Special case of User-user field, there is some confusion here as
@@ -106,14 +115,39 @@ EXTERN int ooQ931Decode
                However, at present we assume it is always 2 bytes until we find
                something that breaks it.
             */
-            len <<= 8;
-            len |= data[offset++];
 
-            /* we also have a protocol discriminator, which we ignore */
-            offset++;
+            /*
+             * Advance offset to point to the second byte of the length
+             * and make sure it's still in-bounds.
+             */
+            if (++offset >= length) {
+                return Q931_E_INVLENGTH;
+            }
+            /*
+             * Swap the bytes.
+             * Yes, this assumes we're running on a little-endian system but this is
+             * the original code from Objective Systems.
+             */
+            len <<= 8;
+            len |= data[offset];
+
+            /*
+             * We also have a nested protocol discriminator, which we ignore.
+             * Advance offset to point to the nested discriminator
+             * and make sure it's still in-bounds.
+             */
+            if (++offset >= length) {
+                return Q931_E_INVLENGTH;
+            }
             len--;
          }
 
+         /*
+          * Advance offset to point to the first data byte.
+          * We check bounds below.
+          */
+         ++offset;
+
          /* watch out for negative lengths! (ED, 11/5/03) */
          if (len < 0) {
             return Q931_E_INVLENGTH;