|
|
|
#521: [PATCH] Avoid uninitialized data in random buffer
|
|
|
|
X
Ticket metadata
|
|
|
X
The Basics
|
|
| Id: |
521 |
| Status: |
resolved |
| Left: |
0 min |
| Priority: |
0/0 |
| Queue: |
OpenSSL-Bugs |
|
|
X
People
|
|
| Owner: |
jaenicke
<lutz@lutz-jaenicke.de>
|
| Requestors: |
daniel.brahneborg@infoflex.se
|
| Cc: |
|
| AdminCc: |
|
|
|
|
X
Links
|
|
| Depends on: |
|
| Depended on by: |
|
| Parents: |
|
| Children: |
|
| Refers to: |
|
| Referred to by: |
|
|
|
X
More about Daniel Brahneborg
|
|
Comments about this user:
No comment entered about this user
This user's 10 highest priority tickets:
Groups this user belongs to:
|
|
|
|
X
History
|
Display mode:[Brief headers] [Full headers]
|
| # |
|
Fri Feb 28 10:28:30 2003 |
daniel.brahneborg@infoflex.se - Ticket created
|
|
|
|
Hi,
I'm using Valgrind to debug a program that uses the OpenSSL
libraries, and got warnings about uninitialized data in the
function RSA_padding_add_PKCS1_type_2(), on the line with
"} while (*p == '\0');" (line 171 in version 0.9.7a). The
following patch ensures that the data is always modified,
something that the bytes() method obviously fails to do.
--- openssl-0.9.7a/crypto/rand/rand_lib.c Thu Jan 30 18:37:45 2003
+++ openssl-0.9.7a-safe/crypto/rand/rand_lib.c Wed Feb 26 13:48:27 2003
@@ -154,6 +154,7 @@
int RAND_bytes(unsigned char *buf, int num)
{
const RAND_METHOD *meth = RAND_get_rand_method();
+ memset(buf, 0, num);
if (meth && meth->bytes)
return meth->bytes(buf,num);
return(-1);
/Basic
______________________________________________________________________
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager majordomo@openssl.org
|
| # |
|
Fri Feb 28 11:10:07 2003 |
larsch@trustcenter.de - Correspondence added
|
|
|
|
Daniel Brahneborg via RT wrote:
> Hi,
>
> I'm using Valgrind to debug a program that uses the OpenSSL
> libraries, and got warnings about uninitialized data in the
> function RSA_padding_add_PKCS1_type_2(), on the line with
> "} while (*p == '\0');" (line 171 in version 0.9.7a). The
> following patch ensures that the data is always modified,
> something that the bytes() method obviously fails to do.
If it's a bug in bytes() why do you change RAND_bytes(),
wouldn't it be more appropriate to patch the bytes() function
of the correspondig RAND_METHOD ? RAND_bytes() is only a
wrapper function to call the bytes() function from the
RAND_METHOD object (if existing).
Regards,
Nils
|
| # |
|
Fri Feb 28 11:34:18 2003 |
daniel.brahneborg@infoflex.se - Correspondence added
|
|
|
|
Nils Larsch via RT wrote:
> Daniel Brahneborg via RT wrote:
>>I'm using Valgrind to debug a program that uses the OpenSSL
>>libraries, and got warnings about uninitialized data in the
>>function RSA_padding_add_PKCS1_type_2(), on the line with
>>"} while (*p == '\0');" (line 171 in version 0.9.7a). The
>>following patch ensures that the data is always modified,
>>something that the bytes() method obviously fails to do.
>
> If it's a bug in bytes() why do you change RAND_bytes(),
> wouldn't it be more appropriate to patch the bytes() function
> of the correspondig RAND_METHOD ? RAND_bytes() is only a
> wrapper function to call the bytes() function from the
> RAND_METHOD object (if existing).
Obviously patching the real bytes() is the proper way to
do it, but since I didn't have the time to further examine
where the uninitialized variable came from, patching
RAND_bytes() became the easy (and simple) solution for me.
At lines 467-469 in crypto/rand/md_rand.c is an interesting
thing:
#ifndef PURIFY
MD_Update(&m,buf,j); /* purify complains */
#endif
That is the code that causes the problem (I just verified
it with Valgrind). Does it have any bad side affects to
always skip that code? Since both Purify and Valgrind is
unhappy with that function call, something must be wrong
with it.
/Basic
|
| # |
|
Thu Mar 13 17:56:30 2003 |
geoff@geoffthorpe.net - Correspondence added
|
|
|
|
I think there's we need to create a FAQ entry about this ...
* Daniel Brahneborg (daniel.brahneborg@infoflex.se) wrote:
[snip]
> At lines 467-469 in crypto/rand/md_rand.c is an interesting
> thing:
>
> #ifndef PURIFY
> MD_Update(&m,buf,j); /* purify complains */
> #endif
>
> That is the code that causes the problem (I just verified
> it with Valgrind). Does it have any bad side affects to
> always skip that code? Since both Purify and Valgrind is
> unhappy with that function call, something must be wrong
> with it.
No, it's fine - the problem is Purify and Valgrind assume all use of
uninitialised data is inherently bad, whereas a PRNG implementation has
nothing but positive (or more correctly, non-negative) things to say
about the idea.
Why do you think the "#ifndef PURIFY" logic is there?
If you're going to run an openssl-based app under instrumentation and
*look* for uses of uninitialised data, add "-DPURIFY" to your configure
line. Please also search the archives for words like "Valgrind",
"Purify", "uninitialised memory", etc. This has come up many times
before.
Cheers,
Geoff
--
Geoff Thorpe
geoff@openssl.org
http://www.openssl.org/
|
| # |
|
Thu Sep 20 09:38:48 2007 |
jaenicke - Taken
|
|
|
|
|
| # |
|
Thu Sep 20 09:38:52 2007 |
jaenicke - Status changed from 'new' to 'open'
|
|
|
|
|
| # |
|
Thu Sep 20 09:40:54 2007 |
jaenicke - Correspondence added
|
|
|
|
A respective compile time macro "PEDANTIC" (to be added to the C flags
as "-DPEDANTIC")
has been added for OpenSSL 0.9.8f. The behavior has been clarified in
the manual page
and the FAQ
|
| # |
|
Thu Sep 20 09:40:56 2007 |
jaenicke - Status changed from 'open' to 'resolved'
|
|
|
|
|
|
|
|
Time to display: 0.586305
|
|
