I've been testing out the LLVM static analysis tool
<http://clang-analyzer.llvm.org/> on various code bases, and it's lighting up a
particular construct used in OpenSSL.
Let me state my position right up front:
I have no idea if this causes any problems in OpenSSL, but I don't
think this code is doing what the authors intended.
Example of what clang is whinging about (from rsa_eay.c):
547 BIGNUM local_d;
548 BIGNUM *d = NULL;
549
550 if (!(rsa->flags & RSA_FLAG_NO_CONSTTIME))
551 {
552 d = &local_d;
553 BN_with_flags(d, rsa->d, BN_FLG_CONSTTIME);
What is says is:
Line 553: Left operand of '&' is a garbage value.
Here's the relevant definition (from bn.h):
296 /* get a clone of a BIGNUM with changed flags, for *temporary*
use only
297 * (the two BIGNUMs cannot not be used in parallel!) */
298 #define BN_with_flags(dest,b,n) ((dest)->d=(b)->d, \
299 (dest)->top=(b)->top, \
300 (dest)->dmax=(b)->dmax, \
301 (dest)->neg=(b)->neg, \
302 (dest)->flags=(((dest)->flags &
BN_FLG_MALLOCED) \
303 | ((b)->flags &
~BN_FLG_MALLOCED) \
304 | BN_FLG_STATIC_DATA \
305 | (n)))
The problem is on line 302, which (I believe) intends to clear all the bits in
dest->flags except the BN_FLG_MALLOCED bit (0x01). But in the code in
rya_eay.c, that bit is uninitialized.
So, we end up with a temporary BIGNUM where the BN_FLG_MALLOCED flag is garbage.
Again - I do not know if this actually causes any problems.
The same construct occurs in:
bn_gcd.c line 571
crypto/rsa/rsa_gen.c line 173
crypto/rsa/rsa_lib.c line 417
crypto/bn/bn_gcd.c line 546
rsa_eay.c line 767
-- Marshall
Marshall Clow Idio Software <mailto:[email protected]>
A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly
moderated down to (-1, Flamebait).
-- Yu Suzuki
