On 7/06/2014 7:10 PM, Jenny Yung via RT wrote: > Hello, > > We ran parfait on OpenSSL and found the following errors in openssl-1.0.1g: > > 1. Error: Uninitialised memory (CWE 456) > Possible access to uninitialised memory '&num' > at line 267 of > components/openssl/openssl-1.0.1/build/sparcv9-wanboot/crypto/evp/bio_b64.c > in function 'b64_read'. > &num allocated at line 146. > &num uninitialised when ctx->start != 0 at line 221.
Already fixed in the 1.0.1 stable branch so it is already included in 1.0.1h onwards and 1.0.1m is the current recommended version. commit a41d5174e27c99d1caefd76a8e927c814ede509e Author: Dr. Stephen Henson <[email protected]> Date: Tue May 6 14:07:37 2014 +0100 Initialize num properly. PR#3289 PR#3345 (cherry picked from commit 3ba1e406c2309adb427ced9815ebf05f5b58d155) > 2. Error: Null pointer dereference (CWE 476) > Read from null pointer rctx > at line 114 of > components/openssl/openssl-1.0.1/build/sparcv9-wanboot/crypto/ocsp/ocsp_ht.c > in function 'OCSP_REQ_CTX_free'. > Function OCSP_sendreq_new may return constant 'NULL' at line > 171, called at line 491 in function 'OCSP_sendreq _bio'. > Constant 'NULL' passed into function OCSP_REQ_CTX_free, > argument rctx, from call at line 498. > Null pointer introduced at line 171 in function > 'OCSP_sendreq_new'. This indicates a different issue is present - in that the error handling path will leak memory. rctx->iobuf = OPENSSL_malloc(rctx->iobuflen); if (!rctx->iobuf) return 0; So if malloc fails rctx itself isn't freed - so that will leak. That will need to be looked at too. > 3. Error: Null pointer dereference (CWE 476) > Read from null pointer rctx > at line 268 of > components/openssl/openssl-1.0.1/build/sparcv9-wanboot/crypto/ocsp/ocsp_ht.c > in function 'OCSP_sendreq_nbio'. > Function OCSP_sendreq_new may return constant 'NULL' at line > 171, called at line 491 in function 'OCSP_sendreq_bio'. > Constant 'NULL' passed into function OCSP_sendreq_nbio, > argument rctx, from call at line 495. > Null pointer introduced at line 171 in function > 'OCSP_sendreq_new'. Looks good - but missed other issue with memory leak on malloc failure. > 4. Error: Null pointer dereference (CWE 476) > Read from null pointer frag > at line 1175 of > components/openssl/openssl-1.0.1/build/sparcv9-wanboot/ssl/d1_both.c in > function 'dtls1_buffer_message'. > Function dtls1_hm_fragment_new may return constant 'NULL' at > line 189, called at line 1173. > Null pointer introduced at line 189 in function > 'dtls1_hm_fragment_new'. Looks good. > The following changes fixes the errors: > > 2 --- openssl-1.0.1g/crypto/evp/bio_b64.c.~1~ Tue Jun 3 > 14:13:33 2014 > 3 +++ openssl-1.0.1g/crypto/evp/bio_b64.c Tue Jun 3 14:14:23 2014 > 4 @@ -143,7 +143,7 @@ > 5 > 6 static int b64_read(BIO *b, char *out, int outl) > 7 { > 8 - int ret=0,i,ii,j,k,x,n,num,ret_code=0; > 9 + int ret=0,i,ii,j,k,x,n,num=0,ret_code=0; > 10 BIO_B64_CTX *ctx; > 11 unsigned char *p,*q; Already covered in previous commits. > 12 > 13 --- openssl-1.0.1g/crypto/ocsp/ocsp_ht.c.~1~ Tue Jun 3 > 14:15:18 2014 > 14 +++ openssl-1.0.1g/crypto/ocsp/ocsp_ht.c Tue Jun 3 > 14:15:46 2014 > 15 @@ -490,6 +490,9 @@ > 16 > 17 ctx = OCSP_sendreq_new(b, path, req, -1); > 18 > 19 + if (!ctx) > 20 + return NULL; > 21 + > 22 do > 23 { > 24 rv = OCSP_sendreq_nbio(&resp, ctx); Looks reasonable - although I don't think the spin loop there is appropriate - basically with no delay, and no select, this will spin on a non-blocking retry condition (which is meant to make it back to the caller to enter their event loop. That is a broader issue to look at. > 25 --- openssl-1.0.1g/ssl/d1_both.c.~1~ Tue Jun 3 14:16:25 2014 > 26 +++ openssl-1.0.1g/ssl/d1_both.c Tue Jun 3 14:17:26 2014 > 27 @@ -1172,6 +1172,8 @@ > 28 > 29 frag = dtls1_hm_fragment_new(s->init_num, 0); > 30 > 31 + if (!frag) > 32 + return 0; > 33 memcpy(frag->fragment, s->init_buf->data, s->init_num); > 34 > 35 if ( is_ccs) That looks good as a patch. > Can you integrate this into the next release of OpenSSL? Can you re-run parfait against the current release version of OpenSSL for that branch - i.e. 1.0.1m It would also be helpful to see suggested patch as a separate RT issue - so we can discuss and track them individually. Thanks, Tim. ______________________________________________________________________ OpenSSL Project http://www.openssl.org Development Mailing List [email protected] Automated List Manager [email protected]
