# HG changeset patch
# User David Keeler <dkeeler@mozilla.com>
# Date 1455892169 -3600
# Node ID b9a31471759d751a56bf261b24c138c8f5d3925f
# Parent  9e2af044dfa443ccff8587177c8f1b5b7b627f37
bug 1245528 - fix bugs in ASN.1 decoding, r=ryan.sleevi

diff --git a/lib/util/secasn1d.c b/lib/util/secasn1d.c
--- a/lib/util/secasn1d.c
+++ b/lib/util/secasn1d.c
@@ -9,16 +9,18 @@
 
 /* #define DEBUG_ASN1D_STATES 1 */
 
 #ifdef DEBUG_ASN1D_STATES
 #include <stdio.h>
 #define PR_Assert sec_asn1d_Assert
 #endif
 
+#include <limits.h>
+
 #include "secasn1.h"
 #include "secerr.h"
 
 typedef enum {
     beforeIdentifier,
     duringIdentifier,
     afterIdentifier,
     beforeLength,
@@ -1588,28 +1590,63 @@ sec_asn1d_parse_leaf (sec_asn1d_state *s
 
     if (state->pending < len)
 	len = state->pending;
 
     bufLen = len;
 
     item = (SECItem *)(state->dest);
     if (item != NULL && item->data != NULL) {
+	unsigned long offset;
 	/* Strip leading zeroes when target is unsigned integer */
 	if (state->underlying_kind == SEC_ASN1_INTEGER && /* INTEGER   */
 	    item->len == 0 &&                             /* MSB       */
 	    item->type == siUnsignedInteger)              /* unsigned  */
 	{
 	    while (len > 1 && buf[0] == 0) {              /* leading 0 */
 		buf++;
 		len--;
 	    }
 	}
-	PORT_Memcpy (item->data + item->len, buf, len);
-	item->len += len;
+        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) {
+            unsigned long len_in_bits;
+            // 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;
+            }
+            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)
 	state->place = beforeEndOfContents;
 
     return bufLen;
 }
 
@@ -1666,24 +1703,16 @@ sec_asn1d_parse_more_bit_string (sec_asn
 	} else {
 	    /* An empty bit string with no unused bits is OK. */
 	    state->place = beforeEndOfContents;
 	}
 	return 0;
     }
 
     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;
 }
 
 
 /*
  * XXX All callers should be looking at return value to detect
  * out-of-memory errors (and stop!).
  */
@@ -2203,17 +2232,17 @@ sec_asn1d_concat_substrings (sec_asn1d_s
 			? PR_TRUE : PR_FALSE;
 
 	substring = state->subitems_head;
 	while (substring != NULL) {
 	    /*
 	     * 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;
 		return;
 	    }
 	    item_len += substring->len;
 	    substring = substring->next;
 	}