Improve ASN.1 parsing.

- Refactor fromDer algorithm.
- More strictly follow structure lengths.
- Improve remaining data checks to avoid overruns with unchecked buffer get
  calls.
- Improve length sanity checks.
- Allow options for fromDer. Handle old strict call signature.
- Add decodeBinaryStrings flag to control decoding of BIT STRINGs.
- Improve handling of ASN.1 encapsulated in BIT STREAMs. Many parsing failure
  cases eliminated.
- Store "raw" BIT STREAM bytes so toDer() and validate() have access to what
  may have decoded as ASN.1 but was just plain bytes.
- Add tests.

Note that with these changes ASN.1 may still parse BIT STREAMs as ASN.1
even though they are plain bytes. This could happen with signatures.
However, the "raw" data is now available and used when going back to
bytes with toDer(). To avoid such parsing completely will require use of
structural schemas.

lib/asn1.js

@@ -231,7 +231,7 @@ asn1.create = function(tagClass, type, constructed, value, options) {
  *
  * @return the length of the BER-encoded ASN.1 value or undefined.
  */
-var _getValueLength = asn1.getBerValueLength = function(b) {
+asn1.getBerValueLength = function(b) {
   // TODO: move this function and related DER/BER functions to a der.js
   // file; better abstract ASN.1 away from der/ber.
   var b2 = b.getByte();
@@ -253,142 +253,278 @@ var _getValueLength = asn1.getBerValueLength = function(b) {
   return length;
 };
 
+/**
+ * Check the byte buffer has enough bytes. Throws an Error if not.
+ *
+ * @param bytes the byte buffer to parse from.
+ * @param remaining the bytes remaining in the current parsing state.
+ * @param n the number of bytes the buffer must have.
+ */
+function _checkBufferLength(bytes, remaining, n) {
+  if(n > remaining) {
+    var error = new Error('Too few bytes to parse DER.');
+    error.available = bytes.length();
+    error.remaining = remaining;
+    error.requested = n;
+    throw error;
+  }
+}
+
+/**
+ * Gets the length of a BER-encoded ASN.1 value.
+ *
+ * In case the length is not specified, undefined is returned.
+ *
+ * @param bytes the byte buffer to parse from.
+ * @param remaining the bytes remaining in the current parsing state.
+ *
+ * @return the length of the BER-encoded ASN.1 value or undefined.
+ */
+var _getValueLength = function(bytes, remaining) {
+  // TODO: move this function and related DER/BER functions to a der.js
+  // file; better abstract ASN.1 away from der/ber.
+  // fromDer already checked that this byte exists
+  var b2 = bytes.getByte();
+  remaining--;
+  if(b2 === 0x80) {
+    return undefined;
+  }
+
+  // see if the length is "short form" or "long form" (bit 8 set)
+  var length;
+  var longForm = b2 & 0x80;
+  if(!longForm) {
+    // length is just the first byte
+    length = b2;
+  } else {
+    // the number of bytes the length is specified in bits 7 through 1
+    // and each length byte is in big-endian base-256
+    var longFormBytes = b2 & 0x7F;
+    _checkBufferLength(bytes, remaining, longFormBytes);
+    length = bytes.getInt(longFormBytes << 3);
+  }
+  // FIXME: this will only happen for 32 bit getInt with high bit set
+  if(length < 0) {
+    throw new Error('Negative length: ' + length);
+  }
+  return length;
+};
+
 /**
  * Parses an asn1 object from a byte buffer in DER format.
  *
  * @param bytes the byte buffer to parse from.
- * @param strict true to be strict when checking value lengths, false to
+ * @param [strict] true to be strict when checking value lengths, false to
  *          allow truncated values (default: true).
+ * @param [options] object with options or boolean strict flag
+ *          [strict] true to be strict when checking value lengths, false to
+ *            allow truncated values (default: true).
+ *          [decodeBinaryStrings] true to attempt to decode the content of
+ *            BIT STRINGs (not OCTET STRINGs) in strict mode. Note that without
+ *            schema support to understand the data context this can
+ *            erroneously decode values that happen to be valid ASN.1.
+ *            (default: true)
  *
  * @return the parsed asn1 object.
  */
-asn1.fromDer = function(bytes, strict) {
-  if(strict === undefined) {
-    strict = true;
+asn1.fromDer = function(bytes, options) {
+  if(options === undefined) {
+    options = {
+      strict: true,
+      decodeBinaryStrings: true
+    };
+  }
+  if(typeof options === 'boolean') {
+    options = {
+      strict: options,
+      decodeBinaryStrings: true
+    };
+  }
+  if(!('strict' in options)) {
+    options.strict = true;
+  }
+  if(!('decodeBinaryStrings' in options)) {
+    options.decodeBinaryStrings = true;
   }
 
   // wrap in buffer if needed
   if(typeof bytes === 'string') {
     bytes = forge.util.createBuffer(bytes);
   }
 
+  return _fromDer(bytes, bytes.length(), 0, options);
+}
+
+/**
+ * Internal functino to parse an asn1 object from a byte buffer in DER format.
+ *
+ * @param bytes the byte buffer to parse from.
+ * @param remaining the number of bytes remaining for this chunk.
+ * @param depth the current parsing depth.
+ * @param options object with same options as fromDer().
+ *
+ * @return the parsed asn1 object.
+ */
+function _fromDer(bytes, remaining, depth, options) {
+  // temporary storage for consumption calculations
+  var start;
+
   // minimum length for ASN.1 DER structure is 2
-  if(bytes.length() < 2) {
-    var error = new Error('Too few bytes to parse DER.');
-    error.bytes = bytes.length();
-    throw error;
-  }
+  _checkBufferLength(bytes, remaining, 2);
 
   // get the first byte
   var b1 = bytes.getByte();
+  // consumed one byte
+  remaining--;
 
   // get the tag class
   var tagClass = (b1 & 0xC0);
 
   // get the type (bits 1-5)
   var type = b1 & 0x1F;
 
-  // get the value length
-  var length = _getValueLength(bytes);
+  // get the variable value length and adjust remaining bytes
+  start = bytes.length();
+  var length = _getValueLength(bytes, remaining);
+  remaining -= start - bytes.length();
 
   // ensure there are enough bytes to get the value
-  if(bytes.length() < length) {
-    if(strict) {
+  if(length !== undefined && length > remaining) {
+    if(options.strict) {
       var error = new Error('Too few bytes to read ASN.1 value.');
-      error.detail = bytes.length() + ' < ' + length;
+      error.available = bytes.length();
+      error.remaining = remaining;
+      error.requested = length;
       throw error;
     }
-    // Note: be lenient with truncated values
-    length = bytes.length();
+    // Note: be lenient with truncated values and use remaining state bytes
+    length = remaining;
   }
 
   // prepare to get value
   var value;
+  // possible raw value
+  var raw;
 
   // constructed flag is bit 6 (32 = 0x20) of the first byte
   var constructed = ((b1 & 0x20) === 0x20);
-
-  // determine if the value is composed of other ASN.1 objects (if its
-  // constructed it will be and if its a BITSTRING it may be)
-  var composed = constructed;
-  if(!composed && tagClass === asn1.Class.UNIVERSAL &&
-    type === asn1.Type.BITSTRING && length > 1) {
-    /* The first octet gives the number of bits by which the length of the
-      bit string is less than the next multiple of eight (this is called
-      the "number of unused bits").
-
-      The second and following octets give the value of the bit string
-      converted to an octet string. */
-    // if there are no unused bits, maybe the bitstring holds ASN.1 objs
-    var read = bytes.read;
-    var unused = bytes.getByte();
-    if(unused === 0) {
-      // if the first byte indicates UNIVERSAL or CONTEXT_SPECIFIC,
-      // and the length is valid, assume we've got an ASN.1 object
-      b1 = bytes.getByte();
-      var tc = (b1 & 0xC0);
-      if(tc === asn1.Class.UNIVERSAL || tc === asn1.Class.CONTEXT_SPECIFIC) {
-        try {
-          var len = _getValueLength(bytes);
-          composed = (len === length - (bytes.read - read));
-          if(composed) {
-            // adjust read/length to account for unused bits byte
-            ++read;
-            --length;
-          }
-        } catch(ex) {}
-      }
-    }
-    // restore read pointer
-    bytes.read = read;
-  }
-
-  if(composed) {
+  if(constructed) {
     // parse child asn1 objects from the value
     value = [];
     if(length === undefined) {
       // asn1 object of indefinite length, read until end tag
       for(;;) {
+        _checkBufferLength(bytes, remaining, 2);
         if(bytes.bytes(2) === String.fromCharCode(0, 0)) {
           bytes.getBytes(2);
+          remaining -= 2;
           break;
         }
-        value.push(asn1.fromDer(bytes, strict));
+        start = bytes.length();
+        value.push(_fromDer(bytes, remaining, depth + 1, options));
+        remaining -= start - bytes.length();
       }
     } else {
       // parsing asn1 object of definite length
-      var start = bytes.length();
       while(length > 0) {
-        value.push(asn1.fromDer(bytes, strict));
+        start = bytes.length();
+        value.push(_fromDer(bytes, remaining, depth + 1, options));
+        remaining -= start - bytes.length();
         length -= start - bytes.length();
+      }
+    }
+  }
+
+  // determine if a non-constructed value should be decoded as a composed
+  // value that contains other ASN.1 objects. BIT STRINGs (and OCTET STRINGs)
+  // can be used this way.
+  if(value === undefined && options.decodeBinaryStrings &&
+    tagClass === asn1.Class.UNIVERSAL &&
+    // FIXME: OCTET STRINGs not yet supported here
+    // .. other parts of forge expect to decode OCTET STRINGs manually
+    (type === asn1.Type.BITSTRING /*|| type === asn1.Type.OCTETSTRING*/) &&
+    length > 1) {
+    // save read position
+    var savedRead = bytes.read;
+    var savedRemaining = remaining;
+    var unused = 0;
+    if(type === asn1.Type.BITSTRING) {
+      /* The first octet gives the number of bits by which the length of the
+        bit string is less than the next multiple of eight (this is called
+        the "number of unused bits").
+
+        The second and following octets give the value of the bit string
+        converted to an octet string. */
+      _checkBufferLength(bytes, remaining, 1);
+      unused = bytes.getByte();
+      remaining--;
+    }
+    // if all bits are used, maybe the BIT/OCTET STREAM holds ASN.1 objs
+    if(unused === 0) {
+      try {
+        // attempt to parse child asn1 object from the value
+        // (stored in array to signal composed value)
         start = bytes.length();
+        var composed = _fromDer(bytes, remaining, depth + 1, options);
+        var used = start - bytes.length();
+        remaining -= used;
+        if(type == asn1.Type.BITSTRING) {
+          used++;
+        }
+
+        // if the data all decoded and the class indicates UNIVERSAL or
+        // CONTEXT_SPECIFIC then assume we've got an encapsulated ASN.1 object
+        var tc = composed.tagClass;
+        if(used === length &&
+          (tc === asn1.Class.UNIVERSAL || tc === asn1.Class.CONTEXT_SPECIFIC)) {
+          value = [composed];
+          // save raw value
+          var current = bytes.read;
+          bytes.read = savedRead;
+          raw = bytes.bytes(length);
+          bytes.read = current;
+        }
+      } catch(ex) {
       }
     }
-  } else {
-    // asn1 not composed, get raw value
+    if(value === undefined) {
+      // restore read position
+      bytes.read = savedRead;
+      remaining = savedRemaining;
+      raw = undefined;
+    }
+  }
+
+  if(value === undefined) {
+    // asn1 not constructed or composed, get raw value
     // TODO: do DER to OID conversion and vice-versa in .toDer?
 
     if(length === undefined) {
-      if(strict) {
+      if(options.strict) {
         throw new Error('Non-constructed ASN.1 object of indefinite length.');
       }
-      // be lenient and use remaining bytes
-      length = bytes.length();
+      // be lenient and use remaining state bytes
+      length = remaining;
     }
 
     if(type === asn1.Type.BMPSTRING) {
       value = '';
-      for(var i = 0; i < length; i += 2) {
+      for(; length > 0; length -= 2) {
+        _checkBufferLength(bytes, remaining, 2);
         value += String.fromCharCode(bytes.getInt16());
+        remaining -= 2;
       }
     } else {
       value = bytes.getBytes(length);
     }
   }
 
+  // add raw data if available
+  var asn1Options = raw === undefined ? null : {raw: raw};
+
   // create and return asn1 object
-  return asn1.create(tagClass, type, constructed, value);
-};
+  return asn1.create(tagClass, type, constructed, value, asn1Options);
+}
 
 /**
  * Converts the given asn1 object to a buffer of bytes in DER format.