reject non-minimally encoded ASN.1 DER data

split scan_asn1derlength into scan_asn1derlength and scan_asn1derlengthvalue

scan/scan_asn1derlength.3

@@ -14,7 +14,18 @@ scan_asn1derlength never reads more than \fIlen\fR bytes from \fIsrc\fR.  If the
 sequence is longer than that, or the memory area contains an invalid
 sequence, scan_asn1derlength returns 0 and does not touch \fIdest\fR.
 
-The length of the longest ASN.1 DER length sequence is 128 bytes.  In
-practice the largest sequence is sizeof(*dest)+1.
+The length of the longest spec-compliant ASN.1 DER length is 128 bytes,
+but this implementation will return an error if the value does not fit
+into the target integer type.  In practice the largest sequence is
+sizeof(*dest)+1.
+
+This implementation will reject values that are not encoded in the
+minimum amount of bytes.
+
+In addition to reading the length value, this implementation will also
+validate the length value. If the length value is so large that the data
+would not fit in the source buffer, it will return a failure.  If you
+only want to parse the length value without this check, use
+scan_asn1derlengthvalue() instead.
 .SH "SEE ALSO"
-fmt_asn1derlength(3)
+fmt_asn1derlength(3), scan_asn1derlengthvalue(3)

scan/scan_asn1derlength.c

@@ -1,25 +1,30 @@
 #include "scan.h"
 
-size_t scan_asn1derlength(const char* src,size_t len,unsigned long long* length) {
-  const char* orig=src;
-  const char* max=orig+len;
-  if (src>=max) return 0;
-/* If the highest bit of the first byte is clear, the byte is the length.
- * Otherwise the next n bytes are the length (n being the lower 7 bits) */
-  if (*src&0x80) {
-    int chars=*src&0x7f;
-    unsigned long long l=0;
-    while (chars>0) {
-      if (++src>=max) return 0;
-      if (l>(((unsigned long long)-1)>>8)) return 0;	/* catch integer overflow */
-      l=l*256+(unsigned char)*src;
-      --chars;
+size_t scan_asn1derlengthvalue(const char* src,size_t len,unsigned long long* value) {
+  if (len==0 || len>=-(uintptr_t)src) return 0;
+  unsigned char i,c=*src;
+  unsigned long long l;
+  if ((c&0x80)==0) {
+    *value=c&0x7f;
+    return 1;
   }
-    *length=l;
-  } else
-    *length=*src&0x7f;
-  src++;
-  if (src+*length>max) return 0;	/* catch integer overflow */
-  if ((uintptr_t)src+*length<(uintptr_t)src) return 0;	/* gcc 4.1 removes this check without the cast to uintptr_t */
-  return (size_t)(src-orig);
+  /* Highest bit set: lower 7 bits is the length of the length value in bytes. */
+  c&=0x7f;
+  if (!c) return 0;	/* length 0x80 means indefinite length encoding, not supported here */
+  l=(unsigned char)src[1];
+  if (l==0) return 0;	/* not minimally encoded: 0x81 0x00 instead of 0x00 */
+  if (c>sizeof(l)) return 0;	/* too many bytes, does not fit into target integer type */
+  for (i=2; i<=c; ++i)
+    l=l*256+(unsigned char)src[i];
+  if (l<0x7f) return 0;	/* not minimally encoded: 0x81 0x70 instead of 0x70 */
+  *value=l;
+  return i;
+}
+
+size_t scan_asn1derlength(const char* src,size_t len,unsigned long long* value) {
+  unsigned long long l;
+  size_t i=scan_asn1derlengthvalue(src,len,&l);
+  if (l > len-i) return 0;		/* make sure data would fit into buffer */
+  *value=l;
+  return i;
 }

scan/scan_asn1derlengthvalue.3

@@ -0,0 +1,29 @@
+.TH scan_asn1derlength 3
+.SH NAME
+scan_asn1derlengthvalue \- decode an unsigned integer from ASN.1 DER length encoding
+.SH SYNTAX
+.B #include <scan.h>
+
+size_t \fBscan_asn1derlengthvalue\fP(const char *\fIsrc\fR,size_t \fIlen\fR,unsigned long long *\fIdest\fR);
+.SH DESCRIPTION
+scan_asn1derlengthvalue decodes an unsigned integer in ASN.1 DER length encoding
+from a memory area holding binary data.  It writes the decode value in
+\fIdest\fR and returns the number of bytes it read from \fIsrc\fR.
+
+scan_asn1derlength never reads more than \fIlen\fR bytes from \fIsrc\fR.  If the
+sequence is longer than that, or the memory area contains an invalid
+sequence, scan_asn1derlength returns 0 and does not touch \fIdest\fR.
+
+The length of the longest spec-compliant ASN.1 DER length is 128 bytes,
+but this implementation will return an error if the value does not fit
+into the target integer type.  In practice the largest sequence is
+sizeof(*dest)+1.
+
+This implementation will reject values that are not encoded in the
+minimum amount of bytes.
+
+If you need to decode the length value so you can parse actual ASN.1
+tag/length/value structures, please consider using scan_asn1derlength
+instead, as it will do additional checking for you.
+.SH "SEE ALSO"
+fmt_asn1derlength(3), scan_asn1derlengthvalue(3)

scan/scan_asn1dertag.c

@@ -9,6 +9,7 @@ size_t scan_asn1dertag(const char* src,size_t len,unsigned long long* length) {
       *length=l;
       return n+1;
     }
+    if (n==0 && !l) return 0;	// DER says: must be encoded minimally
   }
   return 0;
 }

test/marshal.c

@@ -134,6 +134,22 @@ int main() {
   assert(fmt_asn1dertag(NULL,0xc2)==2);
   zap(); assert(fmt_asn1dertag(buf,0xc2)==2 && byte_equal(buf,3,"\x81\x42_"));
 
+  ull=-1; assert(scan_asn1dertag("\x00_",2,&ull)==1 && ull==0);
+  ull=-1; assert(scan_asn1dertag("\x81\x42_",3,&ull)==2 && ull==0xc2);
+  ull=-1; assert(scan_asn1dertag("\x80\x42_",3,&ull)==0 && ull==-1);	// non-minimal encoding
+  ull=-1; assert(scan_asn1dertag("\x80_",1,&ull)==0 && ull==-1);	// incomplete sequence
+  ull=-1; assert(scan_asn1dertag("\x82\x80_",2,&ull)==0 && ull==-1);	// incomplete sequence
+
+  ull=-1; assert(scan_asn1derlength("\x00_",2,&ull)==1 && ull==0);
+  ull=-1; assert(scan_asn1derlengthvalue("\x81\xc2_",3,&ull)==2 && ull==0xc2);
+  ull=-1; assert(scan_asn1derlengthvalue("\x82\x12\x34_",4,&ull)==3 && ull==0x1234);
+  ull=-1; assert(scan_asn1derlengthvalue("\x82\x00\x34_",4,&ull)==0 && ull==-1);	// non-minimal encoding
+  ull=-1; assert(scan_asn1derlengthvalue("\x81\x12_",3,&ull)==0 && ull==-1);	// non-minimal encoding
+  ull=-1; assert(scan_asn1derlengthvalue("\xff_",1,&ull)==0 && ull==-1);	// incomplete sequence
+  ull=-1; assert(scan_asn1derlengthvalue("\xff_",200,&ull)==0 && ull==-1);	// incomplete sequence
+
+  ull=-1; assert(scan_asn1derlength("\x10_",1,&ull)==0 && ull==-1);	// not enough space in buffer for length
+
   zap(); assert(fmt_strm(buf,"hell","o, worl","d!\n")==14 && byte_equal(buf,15,"hello, world!\n_"));
 
   assert(fmt_escapecharxml(NULL,0xc2)==6);
@@ -326,7 +342,7 @@ int main() {
   }
 
   {
-    char* mmapcopy;
+    const char* mmapcopy;
     FILE* f;
     size_t mlen;
     assert(f=fopen("test/marshal.c","rb"));