On 2012-11-08, at 6:57 AM, Kees Cook via RT wrote: > http://www.viva64.com/en/b/0178/ > > OPENSSL_cleanse is being called with pointer size instead of the buffer > size in some places. > For example crypto/des/des.c: > > void doencryption(void) > ... > static unsigned char *buf=NULL,*obuf=NULL; > ... > OPENSSL_cleanse(buf,sizeof(buf)); > OPENSSL_cleanse(obuf,sizeof(obuf)); > > This is leaving memory uncleared. > > -- > Kees Cook @outflux.net
I wrote a semantic patch (for Coccinelle) which detects and fixes this
kind of bug, see attached `buggy-OPENSSL_cleanse.cocci'. For example,
here is its output for the file crypto/des/des.c:
$ spatch --sp-file buggy-OPENSSL_cleanse.cocci \
openssl-1.0.1c/crypto/des/des.c
...
@@ -666,8 +666,8 @@ void doencryption(void)
if (l) fclose(CKSUM_OUT);
}
problems:
- OPENSSL_cleanse(buf,sizeof(buf));
- OPENSSL_cleanse(obuf,sizeof(obuf));
+ OPENSSL_cleanse(buf, BUFSIZE + 8);
+ OPENSSL_cleanse(obuf, BUFSIZE + 8);
OPENSSL_cleanse(&ks,sizeof(ks));
OPENSSL_cleanse(&ks2,sizeof(ks2));
OPENSSL_cleanse(iv,sizeof(iv));
It seems that one is not the only instance. Try:
$ spatch --sp-file buggy-OPENSSL_cleanse.cocci --dir openssl-1.0.1c \
| tee openssl-1.0.1c-buggy-OPENSSL_cleanse.patch
Attached is that patch, which I cleaned up because of some false
positives and other unwanted stuff. I'm not familiar enough with the
OpenSSL code to judge which parts of it are relevant.
Mansour
buggy-OPENSSL_cleanse.cocci
Description: Binary data
openssl-1.0.1c-buggy-OPENSSL_cleanse.patch
Description: Binary data
