Hi Tim,
Thanks for your feedback!
On 06/ 7/14 03:01 PM, Tim Hudson via RT wrote:
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 <st...@openssl.org>
Date: Tue May 6 14:07:37 2014 +0100
Initialize num properly.
PR#3289
PR#3345
(cherry picked from commit 3ba1e406c2309adb427ced9815ebf05f5b58d155)
I ran it on 1.0.1h and confirmed that it has been fixed. Thanks.
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.
see below
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.
I see -- my change blocks the do-while loop. I'll check this some more.
Do you have any suggestions for now?
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.
I have submitted another RT issue with the patch (after removing the
first part that was fixed in 1.0.1h)
Thanks!
Jenny
Thanks,
Tim.
______________________________________________________________________
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager majord...@openssl.org
