Re: [openssl.org #3403] Null dereference and memory leak reports for openssl-1.0.1h from Facebook's Infer static analyzer

2014-06-16 Thread Peter O'Hearn via RT
Hey Rich Salz, you have correctly inferred that the REPORT part is
automated. The REMARKS are my human non-automated commentary resulting
from looking at the code or at other INFER reports for similar issues;
just a little context.

regards,  Peter


__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   majord...@openssl.org


[openssl.org #3403] Null dereference and memory leak reports for openssl-1.0.1h from Facebook's Infer static analyzer

2014-06-13 Thread Peter O'Hearn via RT
Hello,

these 15 null dereference and memory leak reports, included with comments
below, were found by running

Facebook¹s Infer static analyzer on openssl-1.0.1h.

regards,

 
Peter O¹Hearn
Facebook Static Analysis Tools Team
 

 

1.
File: apps/apps.c

REPORT: Null Dereference in apps.c at line 395
pointer arg-data last assigned on line 392 could be null and is
dereferenced at line 395

 
REMARKS: 
‹ The call to openssl_malloc on 392 could return NULL. openssl_malloc is a
wrapper for malloc.
- Note that on line 202 of same file the return value of openssl_malloc is
checked for NULL before dereferencing. This true many other places in the
codebase

-
2.
File: apps/apps.c

REPORT: Null Dereference in apps/apps.c at line 1545
pointer p last assigned on line 1544 could be null and is dereferenced by
call to BUF_strlcpy() at line 1545

 
REMARKS:
‹ The call to openssl_malloc on 1544 might return NULL.
‹ The call BUF_strlcpy(p,t,len)on 1545 dereferences p: BUF_strlcpy()
derefereces its first argument on line 105 of crypto/buffer/buf_str.c,
where its definition resides.
‹ There are similar issues with the calls to BUF_strlcat on lines 1547 and
1549

 
-
3.
File: apps/ca.c

REPORT: Null Dereference in apps/ca.c at line 2780
pointer revtm last assigned on line 2778 could be null and is dereferenced
at line 2780 

 
REMARKS:
- The definition of X509_gmtime_adj is in crypto/x509/x509_vfy.c. It calls
X509_time_ad which calls X509_time_adj_ex which calls several other things
which can return NULL.
‹ The conditions under which X509_gmtime_adj(NULL, 0) returns null are
somewhat complex.
Calls to X509_gmtime_adj(NULL, 0) are checked for NULL before dereference
elsewhere in the codebase; for example, in crypto/cms/cms_sd.c at line 471.
 
--
4.
File: apps/crl2p7.c

REPORT: Null Dereference in apps/crl2p7.c at line 144
pointer certflst last assigned on line 143 could be null and is
dereferenced by call to sk_push() at line 144

 
REMARKS
- sk_OPENSSL_STRING_new_null() used on line 143 is a wrapper for malloc
which can return NULL
- sk_OPENSSL_STRING_push(certflst,*(++argv)) on line 144 calls sk_push
which dereferences its first argument through st-num on line 246 of
crypto/stack/stack.c
‹ sk_OPENSSL_STRING_new_null() is checked for NULL before dereference at
other places in the codebase, such as in
(app_locks=sk_OPENSSL_STRING_new_null()) == NULL) in crypto/cryptlib.c.

 
---
5.
File: crypto/asn1/a_utctm.c

REPORT: Null Dereference in crypto/asn1/a_utctm.c at line 269
pointer tm last assigned on line 263 could be null and is dereferenced at
line 269 

REMARKS
 
- similar to the gmtime example above in apps/ca.c, time OPENSSL_gmtime
- other calls to OPENSSL_gmtime are checked for NULL before dereferencing,
such as on line 113 of crypto/asn1/a_time.c
   
--
6.
File: crypto/asn1/asn_mime.c

REPORT: Null Dereference in crypto/asn1/asn_mime.c at line 697
pointer headers last assigned on line 669 could be null and is
dereferenced by call to sk_push() at line 697

REMARKS:
‹ sk_MIME_HEADER_new(mime_hdr_cmp) at line 669 is a wrapper for malloc
which can return NULL
‹ sk_MIME_HEADER_push calls sk_push() in crypto/stack/stack.c, which
immediately dereferences its first argument through st-num on line 246 of
crypto/stack/stack.c

--
7.
File: crypto/asn1/t_x509.c

REPORT: Null Dereference in crypto/asn1/t_x509.c at line 478
pointer b last assigned on line 477 could be null and is dereferenced at
line 478

REMARKS
- The test !*b on line 478 will be  anull pointer dereference if b itself
it NULL
‹ Calls to X509_NAME_oneline are checked for NULL before dereference
elsewhere in the codebase; for example, in crypto/threads/mttest.c, line
707


--
8.
File: ssl/d1_both.c

REPORT: Null Dereference in ssl/d1_both.c at line 1184
pointer frag last assigned on line 1182 could be null and is dereferenced
at line 1175

REMARKS
‹ dtls1_hm_fragment_new used on line 1182, and defined in this file, is a
wrapper for malloc which could return NULL


---
9.
File: crypto/asn1/a_gentm.c

REPORT: Memory Leak in crypto/asn1/a_gentm.c at line 232
Memory dynamically allocated to s by call to ASN1_STRING_type_new() at
line 226 is not reachable after line 232

 
REMARKS
‹ if allocation succeeds at line 226 and fails at line 230 then s will be
leaked at line 232 
‹ Infer also reports leaks at lines 237 and 248 stemming from this same
allocation site.
 

10.
File: crypto/asn1/a_utctm.c

REPORT: Memory Leak in crypto/asn1/a_utctm.c at line 207
Memory dynamically allocated to s by call to ASN1_STRING_type_new() at
line 201 is not reachable after line 207

 
REMARKS
Infer also reports leaks at lines 212, 216, and 224 stemming from this
same allocation site.
 
-
11.
File: crypto/asn1/ameth_lib.c

REPORT: Memory Leak in