[gnutls-dev] gnutls_pk.c uninitialised memory
Martijn Koster
mak at greenhills.co.uk
Wed May 19 04:02:45 CEST 2004
Hi all,
I was using valgrind on gnutls-cli-debug, and it found a bug.
I tested gnutls-1.0.8, but that code has not changed in HEAD.
valgrind --tool=memcheck \
`which gnutls-cli-debug` pics.half.ebay.com
reported:
Conditional jump or move depends on uninitialised value(s)
at 0x3C294E16: _gnutls_pkcs1_rsa_encrypt (gnutls_pk.c:122)
by 0x3C2938B1: _gnutls_gen_rsa_client_kx (auth_rsa.c:313)
by 0x3C28D76C: _gnutls_send_client_kx_message (gnutls_kx.c:187)
by 0x3C28846F: _gnutls_handshake_client (gnutls_handshake.c:2035)
by 0x3C2880A7: gnutls_handshake (gnutls_handshake.c:1897)
That refers to the last line of this code:
opaque rnd[3];
/* Read three random bytes that will be
* used to replace the zeros.
*/
if ( (ret=_gnutls_get_random( rnd, 3, GNUTLS_STRONG_RANDOM)) < 0) {
gnutls_assert();
gnutls_afree(edata);
return ret;
}
/* use non zero values for
* the first two.
*/
if (rnd[0]==0) rnd[0] = 0xaf;
if (rnd[1]==0) rnd[1] = 0xae;
if (ps[i] == 0) {
/* If the first one is zero then set it to rnd[0].
* If the second one is zero then set it to rnd[1].
* Otherwise add (mod 256) the two previous ones plus rnd[3], or
use
* rnd[1] if the value == 0.
*/
if (i<2) ps[i] = rnd[i];
else ps[i] = GMAX( rnd[3] + ps[i-1] + ps[i-2], rnd[1]);
rnd is a 3-byte array, and get_random fills it with 3 bytes,
but then in the last line rnd[3] is used, which is outside
the array, and uninitialised.
Current code in CVS has the same thing:
http://cvs.gnupg.org/cgi-bin/viewcvs.cgi/gnutls/lib/gnutls_pk.c?rev=HEAD&cvsroot=GNU+TLS+Library&content-type=text/vnd.viewcvs-markup
I presume that rnd[3] should be rnd[2]? With that change the warning
goes away. But I don't know what this code does, so I don't know if it's
the right fix. I've appended a patch.
Regards,
-- Martijn
--- gnutls_pk.c.orig 2004-05-18 14:58:03.569633016 +0100
+++ gnutls_pk.c 2004-05-18 14:58:27.360016328 +0100
@@ -115,11 +115,11 @@
if (ps[i] == 0) {
/* If the first one is zero then set it to rnd[0].
* If the second one is zero then set it to rnd[1].
- * Otherwise add (mod 256) the two previous ones plus rnd[3], or use
+ * Otherwise add (mod 256) the two previous ones plus rnd[2], or use
* rnd[1] if the value == 0.
*/
if (i<2) ps[i] = rnd[i];
- else ps[i] = GMAX( rnd[3] + ps[i-1] + ps[i-2], rnd[1]);
+ else ps[i] = GMAX( rnd[2] + ps[i-1] + ps[i-2], rnd[1]);
}
}
break;
More information about the Gnutls-devel
mailing list