bug 1245528 - fix bugs in ASN.1 decoding, r=ryan.sleevi

1 files changed, 38 insertions, 11 deletions

diff --git a/lib/util/secasn1d.c b/lib/util/secasn1d.c
index 7a5bcfd03..19a96e7b0 100644
--- a/lib/util/secasn1d.c
+++ b/lib/util/secasn1d.c
@@ -14,6 +14,8 @@
 #define PR_Assert sec_asn1d_Assert
 #endif
 
+#include <limits.h>
+
 #include "secasn1.h"
 #include "secerr.h"
 
@@ -1603,8 +1605,41 @@ sec_asn1d_parse_leaf (sec_asn1d_state *state,
 		len--;
 	    }
 	}
-	PORT_Memcpy (item->data + item->len, buf, len);
-	item->len += len;
+        unsigned long offset = item->len;
+        if (state->underlying_kind == SEC_ASN1_BIT_STRING) {
+            // The previous bit string must have no unused bits.
+            if (item->len & 0x7) {
+                PORT_SetError (SEC_ERROR_BAD_DER);
+                state->top->status = decodeError;
+                return 0;
+            }
+            // If this is a bit string, the length is bits, not bytes.
+            offset = item->len >> 3;
+        }
+        if (state->underlying_kind == SEC_ASN1_BIT_STRING) {
+            // Protect against overflow during the bytes-to-bits conversion.
+            if (len >= (ULONG_MAX >> 3) + 1) {
+                PORT_SetError (SEC_ERROR_BAD_DER);
+                state->top->status = decodeError;
+                return 0;
+            }
+            unsigned long len_in_bits = (len << 3) - state->bit_string_unused_bits;
+            // Protect against overflow when computing the total length in bits.
+            if (UINT_MAX - item->len < len_in_bits) {
+                PORT_SetError (SEC_ERROR_BAD_DER);
+                state->top->status = decodeError;
+                return 0;
+            }
+            item->len += len_in_bits;
+        } else {
+            if (UINT_MAX - item->len < len) {
+                PORT_SetError (SEC_ERROR_BAD_DER);
+                state->top->status = decodeError;
+                return 0;
+            }
+            item->len += len;
+        }
+        PORT_Memcpy (item->data + offset, buf, len);
     }
     state->pending -= bufLen;
     if (state->pending == 0)
@@ -1671,14 +1706,6 @@ sec_asn1d_parse_more_bit_string (sec_asn1d_state *state,
     }
 
     len = sec_asn1d_parse_leaf (state, buf, len);
-    if (state->place == beforeEndOfContents && state->dest != NULL) {
-	SECItem *item;
-
-	item = (SECItem *)(state->dest);
-	if (item->len)
-	    item->len = (item->len << 3) - state->bit_string_unused_bits;
-    }
-
     return len;
 }
 
@@ -2208,7 +2235,7 @@ sec_asn1d_concat_substrings (sec_asn1d_state *state)
 	     * All bit-string substrings except the last one should be
 	     * a clean multiple of 8 bits.
 	     */
-	    if (is_bit_string && (substring->next == NULL)
+	    if (is_bit_string && (substring->next != NULL)
 			      && (substring->len & 0x7)) {
 		PORT_SetError (SEC_ERROR_BAD_DER);
 		state->top->status = decodeError;