Add a function for validating a DER-encoded INTEGER.

net/der/parse_values.cc

Index: net/der/parse_values.cc
diff --git a/net/der/parse_values.cc b/net/der/parse_values.cc
index 445903250fb1c504b78eb83f5742d23c6217d816..e4f13f0c36c2ffb6f5c82202c5388a3eec30c8d5 100644
--- a/net/der/parse_values.cc
+++ b/net/der/parse_values.cc
@@ -125,39 +125,58 @@ bool ParseBoolRelaxed(const Input& in, bool* out) {
   return ParseBoolInternal(in, out, true /* relaxed */);
 }
 
+// ITU-T X.690 section 8.3.2 specifies that an integer value must be encoded
+// in the smallest number of octets. If the encoding consists of more than
+// one octet, then the bits of the first octet and the most significant bit
+// of the second octet must not be all zeroes or all ones.
+bool ParseInteger(const Input& in,
+                  bool* out_negative,
+                  size_t* out_numeric_length) {
+  der::ByteReader reader(in);
+  uint8_t first_byte;
+
+  if (!reader.ReadByte(&first_byte))
+    return false;  // Empty inputs are not allowed.
+
+  size_t numeric_length = in.Length();
+
+  uint8_t second_byte;
+  if (reader.ReadByte(&second_byte)) {
+    if (first_byte == 0) {
+      // The first byte in this case doesn't contribute to the number's value
+      // (just used to determine the sign).
+      numeric_length -= 1;
+    }
+
+    if ((first_byte == 0x00 || first_byte == 0xFF) &&
+        (first_byte & 0x80) == (second_byte & 0x80)) {
+      // Not a minimal encoding.
+      return false;
+    }
+  }
+
+  *out_negative = first_byte & 0x80;

[nharper, 2015/08/14 22:23:41]

I'm not familiar with C++ type conversion rules - does this correctly result in
a bool that is 0 or 1, or will it end up being 0 or 0x80?

[eroman, 2015/08/14 23:19:38]

Done. I changed to an explicit boolean (since MSCV will probably complain
anyway).

+  *out_numeric_length = numeric_length;
+  return true;
+}
+
 bool ParseUint64(const Input& in, uint64_t* out) {
+  // Reject non-minimally encoded numbers, negative numbers, and non-negative
+  // numbers that would overflow the output type.
+  bool negative;
+  size_t numeric_length;
+  if (!ParseInteger(in, &negative, &numeric_length))
+    return false;
+  if (negative || numeric_length > sizeof(*out))
+    return false;
+
   ByteReader reader(in);
-  size_t bytes_read = 0;
   uint8_t data;
   uint64_t value = 0;
-  // Note that for simplicity, this check only admits integers up to 2^63-1.
-  if (in.Length() > sizeof(uint64_t) || in.Length() == 0)
-    return false;
+
   while (reader.ReadByte(&data)) {
-    if (bytes_read == 0 && (data & 0x80)) {
-      return false;
-    }
     value <<= 8;
     value |= data;
-    bytes_read++;
-  }
-  // ITU-T X.690 section 8.3.2 specifies that an integer value must be encoded
-  // in the smallest number of octets. If the encoding consists of more than
-  // one octet, then the bits of the first octet and the most significant bit
-  // of the second octet must not be all zeroes or all ones.
-  // Because this function only parses unsigned integers, there's no need to
-  // check for the all ones case.
-  if (bytes_read > 1) {
-    ByteReader first_bytes_reader(in);
-    uint8_t first_byte;
-    uint8_t second_byte;
-    if (!first_bytes_reader.ReadByte(&first_byte) ||
-        !first_bytes_reader.ReadByte(&second_byte)) {
-      return false;
-    }
-    if (first_byte == 0 && !(second_byte & 0x80)) {
-      return false;
-    }
   }
   *out = value;
   return true;