minimal fix to security issue

From:

Simon Josefsson

Subject:

minimal fix to security issue

Date:

Mon, 19 Mar 2012 11:48:50 +0100

User-agent:

Gnus/5.130003 (Ma Gnus v0.3) Emacs/24.0.94 (gnu/linux)

If you want to patch an earlier version of libtasn1 instead of
upgrading, below is a small patch that does the trick. You can check
whether a library is patched or not by running tests/Test_overflow.c
from version 2.12 on your libtasn1 library (use LD_PRELOAD to force
loading of a particular library).
I want to mention that there were no security problem in the
asn1_get_length_der function. It was working properly and as documented
before. The security problem was the callers not checking that the
returned values were reasonable, i.e., that the output length was less
than or equal to the total length of the buffer. However, fixing all
callers of this function would be a huge amount of work. Instead, we
made asn1_get_length_der return an error code when the situation
occured, to protect callers. This fix could be the wrong thing if some
code out there calls the function with a der_len parameter that is
smaller than the entire DER structure length. However, we are hoping
that is not in any significant use, and that overall security will be
improved by having the function sanity check its output rather than
letting the caller do that. This was a judgement call.
Thanks again to Matthew Hall for reporting the issue and to Nikos for
discussion.
/Simon
diff --git a/lib/decoding.c b/lib/decoding.c
index 8c46ce5..968fa96 100644
--- a/lib/decoding.c
+++ b/lib/decoding.c
@@ -54,12 +54,13 @@ _asn1_error_description_tag_error (ASN1_TYPE node, char
*ErrorDescription)
* Extract a length field from DER data.
*
* Returns: Return the decoded length value, or -1 on indefinite
- * length, or -2 when the value was too big.
+ * length, or -2 when the value was too big to fit in a int, or -4
+ * when the decoded length value plus @len would exceed @der_len.
**/
signed long
asn1_get_length_der (const unsigned char *der, int der_len, int *len)
{
- unsigned long ans;
+ int ans;
int k, punt;
*len = 0;
@@ -82,7 +83,7 @@ asn1_get_length_der (const unsigned char *der, int der_len,
int *len)
ans = 0;
while (punt <= k && punt < der_len)
{
- unsigned long last = ans;
+ int last = ans;
ans = ans * 256 + der[punt++];
if (ans < last)
@@ -92,10 +93,13 @@ asn1_get_length_der (const unsigned char *der, int der_len,
int *len)
}
else
{ /* indefinite length method */
- ans = -1;
+ *len = punt;
+ return -1;
}
*len = punt;
+ if (ans + *len < ans || ans + *len > der_len)
+ return -4;
return ans;
}
}