From 24d1ccb1b758a57d29fe6f78dc21e14927d54649 Mon Sep 17 00:00:00 2001 From: leitner Date: Tue, 15 Apr 2014 18:52:21 +0000 Subject: [PATCH] reject non-minimally encoded ASN.1 DER data split scan_asn1derlength into scan_asn1derlength and scan_asn1derlengthvalue --- scan/scan_asn1derlength.3 | 17 +++++++++--- scan/scan_asn1derlength.c | 49 +++++++++++++++++++--------------- scan/scan_asn1derlengthvalue.3 | 29 ++++++++++++++++++++ scan/scan_asn1dertag.c | 1 + test/marshal.c | 18 ++++++++++++- 5 files changed, 88 insertions(+), 26 deletions(-) create mode 100644 scan/scan_asn1derlengthvalue.3 diff --git a/scan/scan_asn1derlength.3 b/scan/scan_asn1derlength.3 index 068edd9..2a13779 100644 --- a/scan/scan_asn1derlength.3 +++ b/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) diff --git a/scan/scan_asn1derlength.c b/scan/scan_asn1derlength.c index dfb22f0..7d2ee91 100644 --- a/scan/scan_asn1derlength.c +++ b/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; - } - *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); +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; + } + /* 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; } diff --git a/scan/scan_asn1derlengthvalue.3 b/scan/scan_asn1derlengthvalue.3 new file mode 100644 index 0000000..2ea4265 --- /dev/null +++ b/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 + +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) diff --git a/scan/scan_asn1dertag.c b/scan/scan_asn1dertag.c index 7b13a27..17b6044 100644 --- a/scan/scan_asn1dertag.c +++ b/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; } diff --git a/test/marshal.c b/test/marshal.c index c559ea3..d4e9e9e 100644 --- a/test/marshal.c +++ b/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"));