Error result code check in ./crypto/x509/x509_vfy.c: error return
value can be negative.
(My personal lesson from this: don't wait to see if one of the Top
Dogs bother asking 'hm, shouldn't this change as well?' - I waited for
the O.G., then forgot. And now I'm still not 110% sure if I saw
something the rest didn't or the statistical more probable(?) version
where I am completely and utterly /wrong/. Though when we factor in my
a.r. regarding error checking... Well, enough banter, spin the wheel,
darling...)
Note: I did *not* check if this was done in any stable at the time.
(I've been riding the bleeding edge wave for years now, with no
glaring ill effects.) This is the result of my own code review
following that CVE. X509_verify_cert() is one of the functions which'
return code should be checked for both zero AND negative values; this
was done in one spot, but not in another. This patch is the other.
I now searched back for that CVE as my comment in my own tree only
mentioned the date, and here's the log for the CVS entry at the time
(posted around 11:48 2009/01/07):
-----------------
Log:
Properly check EVP_VerifyFinal() and similar return values
(CVE-2008-5077).
Submitted by: Ben Laurie, Bodo Moeller, Google Security Team
------------------
In case there's a 'HUH?' or 'WTF?' popping up in any brain anywhere:
X509_verify_cert()
is one of the functions which is included in the CVE (and the patch
for it posted from official CVS at 7/jan/2009 with above Log quote).
It can not only return the usual zero(0) value in case of an error /
invalid report, but given its nature, negative return values in case
of errors / invalid reports are possible as well.
This was previously not checked for, leading to the CVE.
The original CVS patch @ 7/1/2009 patches the OpenSSL source tree for
this in one location for X509_verify_cert(), just not in a second
place where X509_verify_cert() was invoked as well.
That is what this single line of change patch today is about: fixing
the checking of the return value at that second spot in case
X509_verify_cert() produces a NEGATIVE return value. Just like the
original CVE fix did at the other spot.
Ergo: this patch should maybe be applied to the other branches as well.
<ducks for cover ;-) >
--
Met vriendelijke groeten / Best regards,
Ger Hobbelt
--------------------------------------------------
web: http://www.hobbelt.com/
http://www.hebbut.net/
mail: g...@hobbelt.com
mobile: +31-6-11 120 978
--------------------------------------------------
--- /home/ger/prj/1original/openssl/openssl/./crypto/x509/x509_vfy.c 2008-10-08 00:55:27.000000000 +0200
+++ ./crypto/x509/x509_vfy.c 2009-04-07 11:07:52.000000000 +0200
@@ -1124,7 +1124,7 @@
/* Verify CRL issuer */
ret = X509_verify_cert(&crl_ctx);
- if (!ret)
+ if (ret <= 0) /* OpenSSL Security Advisory [07-Jan-2009] */
goto err;
/* Check chain is acceptable */
