I ran into a problem with a stack overflow that has me looking at the
crypto\bio\b_print.c code and wondering a few things. Consider the
following routine (note especially the MS_STATIC variable):
int BIO_vprintf (BIO *bio, const char *format, va_list args)
{
int ret;
size_t retlen;
MS_STATIC char hugebuf[1024*10];
char *hugebufp = hugebuf;
size_t hugebufsize = sizeof(hugebuf);
char *dynbuf = NULL;
int ignored;
dynbuf = NULL;
CRYPTO_push_info("doapr()");
_dopr(&hugebufp, &dynbuf, &hugebufsize,
&retlen, &ignored, format, args);
if (dynbuf)
{
ret=BIO_write(bio, dynbuf, (int)retlen);
OPENSSL_free(dynbuf);
}
else
{
ret=BIO_write(bio, hugebuf, (int)retlen);
}
CRYPTO_pop_info();
return(ret);
}
On a non-windows platform, there is a 10k buffer (hugebuf) being
allocated on the stack. That seems excessive and is causing me a stack
overrun. It seems even more excessive when looking at how it is used. It
is passed to _dopr and then to doapr_outch which is defined as follows:
static void
doapr_outch(
char **sbuffer,
char **buffer,
size_t *currlen,
size_t *maxlen,
int c)
{
/* If we haven't at least one buffer, someone has doe a big booboo
*/
assert(*sbuffer != NULL || buffer != NULL);
if (buffer) {
while (*currlen >= *maxlen) {
if (*buffer == NULL) {
assert(*sbuffer != NULL);
if (*maxlen == 0)
*maxlen = 1024;
*buffer = OPENSSL_malloc(*maxlen);
if (*currlen > 0)
memcpy(*buffer, *sbuffer, *currlen);
*sbuffer = NULL;
} else {
*maxlen += 1024;
*buffer = OPENSSL_realloc(*buffer, *maxlen);
}
}
/* What to do if *buffer is NULL? */
assert(*sbuffer != NULL || *buffer != NULL);
}
if (*currlen < *maxlen) {
if (*sbuffer)
(*sbuffer)[(*currlen)++] = (char)c;
else
(*buffer)[(*currlen)++] = (char)c;
}
return;
}
doapr_outch doesn't even use the static buffer if a dynamic buffer is
available so the 10k stack variable is completely unused in this code.
OK, the static buffer can be used in the memcpy, but that code isn't
used since *currlen will be 0 the first time we are called. The second
assert in doapr_outch prevents us from passing a pointer to a NULL
static buffer to _dopr but that assert seems to protect for code that is
never used (i.e. memcpy).
Bottom line, allocating an unused 10k stack variable seems like a bad
idea. I would remove the second assert in doapr_outch and pass a pointer
to a NULL static buffer to _dopr from BIO_vprintf if it were me. At the
very least the static buffer can be very minimal in size.
Can this change be made? Do I need to submit a patch to do it? Or am I
missing something?
Verdon Walker
(801) 861-2633
[EMAIL PROTECTED]
Novell, Inc., the leading provider of Net business solutions
http://www.novell.com
______________________________________________________________________
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
