Skip Menu |
 
Ticket metadata
The Basics
Id: 521
Status: resolved
Priority: 0/
Queue: OpenSSL-Bugs

Custom Fields
Milestone: (no value)
Subsystem: (no value)
Severity: (no value)
Broken in: (no value)

People
Owner: Lutz Jaenicke
Requestors: Daniel Brahneborg
Cc:
AdminCc:

More about the requestors

Daniel Brahneborg

Comments about this user: No comment entered about this user
Groups this user belongs to
  • Unprivileged
  • Everyone

New reminder:
Subject:
Owner:
Due:

Dates
Created: Fri Feb 28 10:28:30 2003
Starts: Not set
Started: Thu Sep 20 09:38:52 2007
Last Contact: Thu Sep 20 09:40:56 2007
Due: Fri Feb 28 10:28:30 2003
Closed: Thu Sep 20 09:40:56 2007
Updated: Thu Sep 20 09:40:57 2007 by Lutz Jaenicke



Date: Wed, 26 Feb 2003 14:25:50 +0100
From: Daniel Brahneborg <daniel.brahneborg@infoflex.se>
To: openssl-dev@openssl.org
Cc: Daniel Brahneborg <danbra@infoflex.se>
Subject: [PATCH] Avoid uninitialized data in random buffer
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
Show quoted text
______________________________________________________________________
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager majordomo@openssl.org
Date: Fri, 28 Feb 2003 11:10:13 +0100
From: Nils Larsch <larsch@trustcenter.de>
To: rt@openssl.org
Subject: Re: [openssl.org #521] [PATCH] Avoid uninitialized data in random buffer
RT-Send-Cc:
Download (untitled) / with headers
text/plain 687b
Daniel Brahneborg via RT wrote:
Show quoted text
> 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
Date: Fri, 28 Feb 2003 11:34:06 +0100
From: Daniel Brahneborg <daniel.brahneborg@infoflex.se>
To: rt@openssl.org
Cc: openssl-dev@openssl.org, Daniel Brahneborg <daniel.brahneborg@infoflex.se>
Subject: Re: [openssl.org #521] [PATCH] Avoid uninitialized data in random buffer
RT-Send-Cc:
Download (untitled) / with headers
text/plain 1.2k
Nils Larsch via RT wrote:
Show quoted text
> 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
Date: Thu, 13 Mar 2003 11:56:17 -0500
From: Geoff Thorpe <geoff@geoffthorpe.net>
Subject: Re: [openssl.org #521] [PATCH] Avoid uninitialized data in random buffer
To: openssl-dev@openssl.org
Cc: rt@openssl.org, Daniel Brahneborg <daniel.brahneborg@infoflex.se>
RT-Send-Cc:
Download (untitled) / with headers
text/plain 1.1k
I think there's we need to create a FAQ entry about this ...

* Daniel Brahneborg (daniel.brahneborg@infoflex.se) wrote:

[snip]

Show quoted text
> 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/
Download (untitled) / with headers
text/plain 185b
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