Dear Openssl developers,
I am a Ph.D. student in the Software Engineering Research Group of
EECS department at Case Western Reserve University, under the
instruction of Prof. Andy Podgurski. In our very recent research, we
applied inter-procedural static program analysis and data mining
technique on the version of 0.9.8g of Openssl project, and we have
found 26 potential bugs (these are a part of bugs we have found), as
indicated at the end of this email. The 26 potential bugs can be
categorized into the following 3 groups:
Category-1: missing of a check of the return value of a function
A function may return an error code such as 0, -1, or NULL to indicate
that an error occurred inside of a function. We’ve found several
potential bugs where a check of the return value is likely to be
missing for certain functions.
Category-2: missing of a check of a parameter of a function
When a parameter of a function is likely to be a NULL-pointer, we need
to first check whether it is NULL or not before dereferencing it.
We've found potential bugs where a check of a parameter of certain
functions is likely to be missing.
Category-3: missing of a function call
This normally happens when a function call is missing in a set of
function calls that always need to be invoked together, for example,
malloc() and free().
The detailed information of each bug is as followed:
Category-1,2 or 3
File Name-the file where the bug occurs
Function Name-the function where the bug occurs
Buggy Code-exact line numbers of the buggy code
Description-description of the bug
Some of the potential bugs are inter-procedural, which cross many
functions. These potential bugs are normally hard to be discovered by
manual effort, and if they are real bugs, it should be valuable to
developers since they are hard to be recognized. Note that for
interprocedural potential bugs, there are several code segments
involved. The info of some code segment will be titled "Code" instead
of "Buggy Code". This indicates that the code is not buggy, but is the
inter-procedural environment of the real location of the bug.
The reason why we did not submit these bugs to the bug tracking system
is that these potential bugs have not triggered any failure, and we
can not be sure whether these potential bugs are real bugs or just bad
code practice. We hope that these potential bugs can help you
recognize some real bugs or inappropriate code practice. *It would
also be greatly appreciated if you can reply to this email after you
have gone over the bugs and tell us whether your have confirmed any of
them*, since these information are really valuable for us for testing
our current method.
If you have any further information, please contact me by email at
[EMAIL PROTECTED]
Thanks!
Raymond
BUG#27
Category: 1 (551)
File Name: /crypto/bn/bn_gf2m.c
Function Name: BN_GF2m_add()
Buggy Code:
297: bn_wexpand(r, at->top);
Description: We found a rule specifying that an error occurs if
bn_wexpand() returns NULL. However, the output of the function is not
checked in the above code.
=====================================================================
BUG#2
Category: 1
File Name: /crypto/ec/ec_mult.c
Function Name: ec_pre_comp_dup()
Buggy Code:
127: CRYPTO_add(&src->references, 1,
CRYPTO_LOCK_EC_PRE_COMP);
File Name: /crypto/dso/dso_lib.c
Function Name: DSO_up_ref()
Buggy Code:
190: CRYPTO_add(&dso->references,1,CRYPTO_LOCK_DSO);
File Name: /crypto/engine/eng_list.c
Function Name: ENGINE_up_ref()
Buggy Code:
429: CRYPTO_add(&e->struct_ref,1,CRYPTO_LOCK_ENGINE);
File Name: /crypto/asn1/x_pubkey.c
Function Name: X509_PUBKEY_get()
Buggy Code:
371: CRYPTO_add(&ret->references, 1, CRYPTO_LOCK_EVP_PKEY);
Description: We found a potential rule specifying that an error occurs
if CRYPTO_add() returns 0. However, the outputs of the functions are
not checked in the above code.
=====================================================================
BUG#3
Category: 1 (537)
File Name: /crypto/conf/conf_mod.c
Function Name: module_load_dso()
Buggy Code:
273: ffunc = (conf_finish_func *)DSO_bind_func(dso,
DSO_mod_finish_name);
274: /* All OK, add module */
275: md = module_add(dso, name, ifunc, ffunc);
Description: We found a rule specifying that an error occurs if
BN_dec_2bn() returns 0. However, the output of the function is not
checked in the above code.
BUG#4
Category: 1 (524)
File Name: /crypto/asn1/x_x509.c
Function Name: X509_keyid_set1()
Correct Code:
120: return ASN1_STRING_set(aux->keyid, id, len);
File Name: /apps /pkcs12.c
Function Name: main()
Baggy Code:
447: X509_keyid_set1(ucert, NULL, 0);
Description: We found a rule specifying that an error occurs if
DSA_generate_key() returns 0. However, the output of the function is
not checked in the above code.
=====================================================================
BUG#5
Category: 1 (524)
File Name: /crypto/asn1/x_x509.c
Function Name: X509_keyid_set1()
Correct Code:
120: return ASN1_STRING_set(aux->keyid, id, len);
File Name: /apps /pkcs12.c
Function Name: main()
Baggy Code:
478: X509_alias_set1(ucert, NULL, 0);
559: X509_alias_set1(sk_X509_value(certs,i), catmp, -1);
File Name: /apps /x509.c
Function Name: main()
Baggy Code:
672: if (alias) X509_alias_set1(x, (unsigned char *)
alias, -1);
Description: We found a rule specifying that an error occurs if
DSA_generate_key() returns 0. However, the output of the function is
not checked in the above code.
=====================================================================
BUG#6
Category: 1 (524)
File Name: /crypto/asn1/x_pubkey.c
Function Name: X509_PKEY_set()
Buggy Code:
212: ASN1_STRING_set(a->parameter->value.sequence, p, i);
File Name: /crypto/evp/evp_pkey.c
Function Name: eckey_pkey2pkcs8()
Buggy Code:
672: ASN1_STRING_set(p8->pkeyalg->parameter->value.sequence, p, i);
File Name: /crypto/asn1/asn1_gen.c
Function Name: asn1_str2type()
Buggy Code:
779: ASN1_STRING_set(atmp->value.asn1_string, str, -1);
Description: We found a rule specifying that an error occurs if
X509_ALGOR_new() returns NULL. However, the output of the function is
not checked in the above code.
=====================================================================
BUG#7
Category: 3 (516)
File Name: /crypto/asn1/a_hdr.c
Function Name: ASN1_HEADER_new()
Buggy Code:
105: M_ASN1_New(ret->header,M_ASN1_OCTET_STRING_new); //NULL
File Name: /crypto/asn1/x_pkey.c
Function Name: X509_PKEY_new()
Buggy Code:
115: M_ASN1_New(ret->enc_pkey,M_ASN1_OCTET_STRING_new);
Description: We found a rule specifying that the object generated by
ASN1_generate_v3() should be released by calling ANS1_TYPE_free().
However, the object generated by ASN1_generate_v3() in the above code
is not released by calling ASN1_TYPE_free(). (The object is not
released in the ancestor functions of do_othername()).
=====================================================================
BUG#8
Category: 2 (469)
File Name: /crypto/bn/bn_print.c
Function Name: BN_dec2bn()
Buggy Code:
281: BN_add_word(ret,l);
File Name: /crypto/pkcs12/p12_key.c
Function Name: PKCS12_key_gen_uni()
Buggy Code:
180: BN_add_word(Bp11,l);
Description: We found a rule requiring that the second parameter of
level_add_node() be an non-NULL object. However, the second parameter
of level_add_node() in the above code might be NULL.
=====================================================================
BUG#9
Category: 1 (470)
File Name: /crypto/conf/conf_def.c
Function Name: str_copy()
Buggy Code:
632: BUF_MEM_grow_clean(buf,(strlen(p)+buf-
>length-(e-from))); // 0
Description: We found a rule specifying that an error occurs if
OBJ_sn2nid() returns the constant “NID_undef”. However, the output of
the function is not checked in the above code.
=====================================================================
BUG#10
Category: 1 (493)
File Name: /crypto/conf/def_load_bio.c
Function Name: def_load_bio()
Code:
266: BIO_gets(in, p, CONFBUFSIZE-1);
File Name: /crypto/txt_db/txt_db.c
Function Name: TXT_DB_read()
Code:
114: BIO_gets(in,&(buf->data[offset]),size-offset);
=====================================================================
BUG#11
Category: 1 (501)
File Name: /crypto/asn1/evp_asn1.c
Function Name: ASN1_TYPE_set_int_octetstring()
Buggy Code:
110: ASN1_INTEGER_set(&in,num);
Category: 1 (501)
File Name: /crypto/x509/x509_set.c
Function Name: X509_set_version()
Buggy Code:
74: return(ASN1_INTEGER_set(x->cert_info-
>version,version));
File Name: /demos/x509/selfsign.c
Function Name: mkcert()
Buggy Code:
95: X509_set_version(x,2);
File Name: /demos/selfsign.c
Function Name: mkitt()
Buggy Code:
109: X509_set_version(x,3);
File Name: /apps/x509.c
Function Name: x509_certify()
Buggy Code:
1174: X509_set_version(x,2);
File Name: /apps/x509.c
Function Name: sign ()
Buggy Code:
1251: X509_set_version(x,2);
File Name: /crypto/pkcs7/pk7_lib.c
Function Name: PKCS7_set_type
Buggy Code:
187: ASN1_INTEGER_set(p7->d.signed_and_enveloped->version,1);
Description: We found a rule specifying that an error occurs if
ssl_cipher_get_evp() returns 0. However, the output of the function is
not checked in the above code.
=====================================================================
BUG#11
Category: 1 (119)
File Name: /crypto/x509v3/v3_pci.c
Function Name: process_pci_value()
Buggy Code:
148: BIO *b = BIO_new_file(val->value + 5, "r");
149: if (!b)
150: /// miss the calling of BIO_free(b);
{
Description: We found a rule specifying that an error occurs if
ssl_cipher_list_to_bytes() returns 0. However, the output of the
function is not checked in the above code.
=====================================================================
BUG#12
Category: 1 (125)
File Name: /crypto/bn/bntest.c
Function Name: test_sqr()
Buggy Code:
690: a.neg=rand_neg();
691: BN_sqr(&c,&a,ctx);
692: if (bp != NULL)
Description: We found a rule specifying that an error occurs if BN_sqr
() returns 0. However, the output of the function is not checked in
the above code.
=====================================================================
BUG#13
Category: 1 (131)
File Name: /crypto/dh/dh_check.c
Function Name: DH_check_pub_key()
Buggy Code:
133: BN_copy(q,dh->p);
134: BN_sub_word(q,1);
135: if (BN_cmp(pub_key,q) >= 0)
Description: We found a rule specifying that an error occurs if
BN_sub_word() returns 0. However, the output of the function is not
checked in the above code.
=====================================================================
BUG#14
Category: 1 (133)
File Name: /crypto/bn/bntest.c
Function Name: test_mont()
Buggy Code:
753: BN_mod_mul_montgomery(&c,&A,&B,mont,ctx);/**/
754: BN_from_montgomery(&A,&c,mont,ctx);/**/
755: if (bp != NULL)
Description: We found a rule specifying that an error occurs if
BN_from_montgomery() returns 0. However, the output of the function is
not checked in the above code.
=====================================================================
BUG#15
Category: 1 (141)
File Name: /apps/speed.c
Function Name: speed_main ()
Buggy Code:
2441: for (count=0,run=1; COND(ecdh_c[j][0]); count++)
2442: {
2443: ECDH_compute_key(secret_a, outlen,
2444: EC_KEY_get0_public_key(ecdh_b[j]),
2445: ecdh_a[j], kdf);
2446: }
Description: We found a rule specifying that an error occurs if
ECDH_compute_key() returns 0. However, the output of the function is
not checked in the above code.
=====================================================================
BUG#16
Category: 1 (161)
File Name: /crypto/x509/509_v3.c
Function Name: X509_EXTENSION_set_object()
Buggy Code:
235: ASN1_OBJECT_free(ex->object);
236: ex->object=OBJ_dup(obj);
237: return(1);
238: }
File Name: /crypto/x509/509_att.c
Function Name: X509_ATTRIBUTE_set1_object()
Buggy Code:
265: ASN1_OBJECT_free(attr->object);
266: attr->object=OBJ_dup(obj);
267: return(1);
Description: We found a rule specifying that an error occurs if OBJ_dup
() returns NULL. However, the outputs of the function are not checked
in the above code.
=====================================================================
BUG#17
Category: 1 (163)
File Name: /crypto/bn/bntest.c
Function Name: test_lshift1()
Buggy Code:
1890: {
1891: BN_lshift1(b,a);
1892: if (bp != NULL)
Description: We found a rule specifying that an error occurs if
BN_lshift1() returns 0. However, the output of the function is not
checked in the above code.
=====================================================================
BUG#18
Category: 1 (164)
File Name: /crypto/bn/bn_print.c
Function Name: BN_dec_2bn()
Buggy Code:
278: if (++j == BN_DEC_NUM)
279: {
280: BN_mul_word(ret,BN_DEC_CONV);
281: BN_add_word(ret,l);
File Name: /crypto/bn/bntestc
Function Name: test_div_word()
Buggy Code:
534: BN_mul_word(&b,s);
535: BN_add_word(&b,r);
Description: We found a rule specifying that an error occurs if
BN_mul_word() returns 0. However, the outputs of the function are not
checked in the above code.
=====================================================================
BUG#19
Category: 1 (175)
File Name: /crypto/dsa/dsatest.c
Function Name: main()
Buggy Code:
210: DSA_generate_key(dsa);
211: DSA_sign(0, str1, 20, sig, &siglen, dsa);
212: if (DSA_verify(0, str1, 20, sig, siglen, dsa) == 1)
216: DSA_generate_key(dsa);
217: DSA_sign(0, str1, 20, sig, &siglen, dsa);
218: if (DSA_verify(0, str1, 20, sig, siglen, dsa) == 0)
Description: We found a rule specifying that an error occurs if
DSA_sign() returns 0. However, the output of the function is not
checked in the above code.
=====================================================================
BUG#20
Category: 1 (191)
File Name: /crypto/x509/x509rset.c
Function Name: X509_REQ_set_subject_name()
Code:
74: if ((x == NULL) || (x->req_info == NULL)) return(0);
75: return(X509_NAME_set(&x->req_info->subject,name));
File Name: /crypto/x509/ca.c
Function Name: do_body()
Buggy Code:
1698: X509_REQ_set_subject_name(req,n);
1699: req->req_info->enc.modified = 1;
Description: We found a rule specifying that an error occurs if
X509_NAME_set() returns 0. The function X509_REQ_set_subject_name()
returns the output of X509_NAME_set(). Thus it is inferred that en
error occurs if X509_REQ_set subject_name() returns 0. However, the
output of X509_REQ_set subject_name() is not checked in the above
code.
=====================================================================
BUG#21
Category: 1 (455)
File Name: /ssl/ssl_ciph.c
Function Name: ssl_load_ciphers()
Buggy Code:
229: ssl_cipher_methods[SSL_ENC_DES_IDX]=
230: EVP_get_cipherbyname(SN_des_cbc);
231: ssl_cipher_methods[SSL_ENC_3DES_IDX]=
232: EVP_get_cipherbyname(SN_des_ede3_cbc);
233: ssl_cipher_methods[SSL_ENC_RC4_IDX]=
234: EVP_get_cipherbyname(SN_rc4);
235: ssl_cipher_methods[SSL_ENC_RC2_IDX]=
236: EVP_get_cipherbyname(SN_rc2_cbc);
238: ssl_cipher_methods[SSL_ENC_IDEA_IDX]=
239: EVP_get_cipherbyname(SN_idea_cbc);
243: ssl_cipher_methods[SSL_ENC_AES128_IDX]=
244: EVP_get_cipherbyname(SN_aes_128_cbc);
245: ssl_cipher_methods[SSL_ENC_AES256_IDX]=
246: EVP_get_cipherbyname(SN_aes_256_cbc);
247: ssl_cipher_methods[SSL_ENC_CAMELLIA128_IDX]=
248: EVP_get_cipherbyname(SN_camellia_128_cbc);
249: ssl_cipher_methods[SSL_ENC_CAMELLIA256_IDX]=
250: EVP_get_cipherbyname(SN_camellia_256_cbc);
251: ssl_cipher_methods[SSL_ENC_SEED_IDX]=
252: EVP_get_cipherbyname(SN_seed_cbc);
Description: We found a rule specifying that an error occurs if
EVP_get_cipherbyname() returns NULL. However, the outputs of the
function are not checked in the above code.
=====================================================================
BUG#22
Category: 1 (454)
File Name: /ssl/ssl_ciph.c
Function Name: ssl_load_ciphers()
Buggy Code:
254: ssl_digest_methods[SSL_MD_MD5_IDX]=
255: EVP_get_digestbyname(SN_md5);
256: ssl_digest_methods[SSL_MD_SHA1_IDX]=
257: EVP_get_digestbyname(SN_sha1);
258: }
Description: We found a rule specifying that an error occurs if
EVP_get_digestbyname() returns NULL. However, the outputs of the
function are not checked in the above code.
=====================================================================
BUG#23
Category: 1 (451)
File Name: /crypto/bn/bntest.c
Function Name: test_exp()
Buggy Code:
1046: for( ; !BN_is_zero(b) ; BN_sub(b,b,one))
1047: BN_mul(e,e,a,ctx);
File Name: /crypto/bn/bntest.c
Function Name: test_mul()
Buggy Code:
647: BN_mul(&c,&a,&b,ctx);
648: if (bp != NULL)
File Name: /crypto/bn/bntest.c
Function Name: test_mod_mul()
Buggy Code:
892: BN_mul(d,a,b,ctx);
893: BN_sub(d,d,e);
File Name: /crypto/bn/bntest.c
Function Name: test_div_recp()
Buggy Code:
598: BN_mul(&e,&d,&b,ctx);
599: BN_add(&d,&e,&c);
Description: We found a rule specifying that an error occurs if BN_mul
() returns 0. However, the outputs of the function are not checked in
the above code.
=====================================================================
BUG#24
Category: 1 (449)
File Name: /apps/speed.c
Function Name: main()
Buggy Code:
2409: secret_size_a = ECDH_compute_key(secret_a, outlen,
2410: EC_KEY_get0_public_key(ecdh_b[j]),
2411: ecdh_a[j], kdf);
2412: secret_size_b = ECDH_compute_key(secret_b, outlen,
2413: EC_KEY_get0_public_key(ecdh_a[j]),
2414: ecdh_b[j], kdf);
2443: ECDH_compute_key(secret_a, outlen,
2444: EC_KEY_get0_public_key(ecdh_b[j]),
2445: ecdh_a[j], kdf);
File Name: /crypto/ecdh/ecdhtest.c
Function Name: test_ecdh_curve()
Buggy Code:
206: abuf=(unsigned char *)OPENSSL_malloc(alen);
207: aout=ECDH_compute_key(abuf,alen,EC_KEY_get0_public_key(b),a,
KDF1_SHA1);
223: bbuf=(unsigned char *)OPENSSL_malloc(blen);
224: out=ECDH_compute_key(bbuf,blen,EC_KEY_get0_public_key
(a),b,KDF1_SHA1);
Description: We found a rule specifying that an error occurs if
EC_KEY_get0_public_key() returns NULL. However, the outputs of the
function are not checked in the above code.
=====================================================================
BUG#24
Category: 1 (446)
File Name: /crypto/bn/exptest.c
Function Name: main()
Buggy Code:
112: RAND_bytes(&c,1);
113: c=(c%BN_BITS)-BN_BITS2;
116: RAND_bytes(&c,1);
117: c=(c%BN_BITS)-BN_BITS2;
Description: We found a rule specifying that an error occurs if
RAND_bytes() returns -1. However, the outputs of the function are not
checked in the above code.
=====================================================================
BUG#25
Category: 1 (445)
File Name: /apps/ca.c
Function Name: do_body()
Buggy Code:
1992: if (strcmp(startdate,"today") == 0)
1993: X509_gmtime_adj(X509_get_notBefore(ret),0);
1996: if (enddate == NULL)
1997: X509_gmtime_adj(X509_get_notAfter(ret),(long)60*60*24*days);
File Name: /apps/ca.c
Function Name: main()
Buggy Code:
1390: X509_gmtime_adj(tmptm,0);
1392: X509_gmtime_adj(tmptm,(crldays*24+crlhours)*60*60);
File Name: /apps/x509.c
Function Name: main()
Buggy Code:
624: X509_gmtime_adj(X509_get_notBefore(x),0);
625: X509_gmtime_adj(X509_get_notAfter(x),(long)60*60*24*days);
Description: We found a rule specifying that an error occurs if
X509_gmtime_adj() returns NULL. However, the outputs of the function
are not checked in the above code.
=====================================================================
BUG#26
Category: 1 (442)
File Name: /crypto/stack/stack.c
Function Name: sk_new_null()
Code:
115: STACK *sk_new_null(void)
117: return sk_new((int (*)(const char * const *, const
char * const *))0);
File Name: /apps/crl2p7.c
Function Name: main()
Buggy Code:
144: if (!certflst) certflst = sk_new_null();
File Name: /apps/ocsp.c
Function Name: main()
Buggy Code:
144: reqnames = sk_new_null();
File Name: /apps/pkcs12.c
Function Name: main()
Buggy Code:
235: if (!canames) canames = sk_new_null();
File Name: /apps/engine.c
Function Name: main()
Buggy Code:
350: STACK *engines = sk_new_null();
351: STACK *pre_cmds = sk_new_null();
352: STACK *post_cmds = sk_new_null();
Description: We found a rule specifying that an error occurs if sk_new
() returns NULL. The function sk_new_null() returns the object
generated by sk_new() directly. Thus, it is inferred that an error
occurs if sk_new_null() returns NULL. However, the outputs of
sk_new_null() are not checked in the above code.
=====================================================================
______________________________________________________________________
OpenSSL Project http://www.openssl.org
Development Mailing List [email protected]
Automated List Manager [EMAIL PROTECTED]
