Add a function for validating a DER-encoded INTEGER.

net/der/parse_values.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/logging.h"
 #include "base/numerics/safe_math.h"
 #include "net/der/parse_values.h"
 
 namespace net {
 
@@ skipping 93 matching lines @@
           return false;
       }
       break;
     default:
       NOTREACHED();
       return false;
   }
   return true;
 }
 
+// Returns the number of bytes of numeric precision in a DER encoded INTEGER
+// value. |in| must be a valid DER encoding of an INTEGER for this to work.
+//
+// Normally the precision of the number is exactly in.Length(). However when
+// encoding positive numbers using DER it is possible to have a leading zero
+// (to prevent number from being interpreted as negative).
+//
+// For instance a 160-bit positive number might take 21 bytes to encode. This
+// function will return 20 in such a case.
+size_t GetNumberOfBytesInInteger(const Input& in) {

[davidben, 2015/08/18 19:44:15]

I would maybe call this GetUnsignedIntegerLength or something like that.
Something that explicitly says "unsigned" since you're expressly not trying to
allow negative numbers. (A signed 2^160-1's precision in bytes arguably is 21.)

[eroman, 2015/08/18 19:54:13]

Done.

+  der::ByteReader reader(in);
+  uint8_t first_byte;
+  if (!reader.ReadByte(&first_byte))
+    return 0;  // Not valid DER  as |in| was empty.
+
+  if (first_byte == 0 && in.Length() > 1)
+    return in.Length() - 1;
+  return in.Length();
+}
+
 }  // namespace
 
 bool ParseBool(const Input& in, bool* out) {
   return ParseBoolInternal(in, out, false /* relaxed */);
 }
 
 // BER interprets any non-zero value as true, while DER requires a bool to
 // have either all bits zero (false) or all bits one (true). To support
 // malformed certs, we recognized the BER encoding instead of failing to
 // parse.
 bool ParseBoolRelaxed(const Input& in, bool* out) {
   return ParseBoolInternal(in, out, true /* relaxed */);
 }
 
-bool ParseUint64(const Input& in, uint64_t* out) {
-  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)) {
+// 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 IsValidInteger(const Input& in, bool* out_negative) {

[davidben, 2015/08/18 19:44:16]

Nit: negative to match the header, now that you don't have the
numeric_length/out_numeric_length problem.

(Bleh. I really really wish Chromium used the "out_" convention. It avoids that
problem so nicely.)

[eroman, 2015/08/18 19:54:13]

Done.

+  der::ByteReader reader(in);
+  uint8_t first_byte;
+
+  if (!reader.ReadByte(&first_byte))
+    return false;  // Empty inputs are not allowed.
+
+  uint8_t second_byte;
+  if (reader.ReadByte(&second_byte)) {
+    if ((first_byte == 0x00 || first_byte == 0xFF) &&
+        (first_byte & 0x80) == (second_byte & 0x80)) {
+      // Not a minimal encoding.
       return false;
     }
   }
+
+  *out_negative = (first_byte & 0x80) == 0x80;
+  return true;
+}
+
+bool ParseUint64(const Input& in, uint64_t* out) {
+  // Reject non-minimally encoded numbers and negative numbers.
+  bool negative;
+  if (!IsValidInteger(in, &negative) || negative)
+    return false;
+
+  // Reject (non-negative) integers whose value would overflow the output type.
+  if (GetNumberOfBytesInInteger(in) > sizeof(*out))
+    return false;
+
+  ByteReader reader(in);
+  uint8_t data;
+  uint64_t value = 0;
+
+  while (reader.ReadByte(&data)) {
+    value <<= 8;
+    value |= data;
+  }
   *out = value;
   return true;
 }
 
 BitString::BitString(const Input& bytes, uint8_t unused_bits)
     : bytes_(bytes), unused_bits_(unused_bits) {
   DCHECK_LT(unused_bits, 8);
   DCHECK(unused_bits == 0 || bytes.Length() != 0);
 }
 
@@ skipping 135 matching lines @@
     return false;
   if (!ValidateGeneralizedTime(time))
     return false;
   *value = time;
   return true;
 }
 
 }  // namespace der
 
 }  // namespace net