Clarify logic in BIO_*printf functions

The static function dynamically allocates an output buffer if the output
grows larger than the static buffer that is normally used. The original
logic implied that |currlen| could be greater than |maxlen| which is
incorrect (and if so would cause a buffer overrun). Also the original
logic would call OPENSSL_malloc to create a dynamic buffer equal to the
size of the static buffer, and then immediately call OPENSSL_realloc to
make it bigger, rather than just creating a buffer than was big enough in
the first place. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot
Oberoi (Int3 Solutions) for reporting this issue.

Reviewed-by: Andy Polyakov <appro@openssl.org>
This commit is contained in:
Matt Caswell 2015-04-27 15:41:03 +01:00
parent b86d7dca69
commit 9d9e37744c

View File

@ -704,32 +704,29 @@ doapr_outch(char **sbuffer,
/* If we haven't at least one buffer, someone has doe a big booboo */ /* If we haven't at least one buffer, someone has doe a big booboo */
assert(*sbuffer != NULL || buffer != NULL); assert(*sbuffer != NULL || buffer != NULL);
if (buffer) { /* |currlen| must always be <= |*maxlen| */
while (*currlen >= *maxlen) { assert(*currlen <= *maxlen);
if (*buffer == NULL) {
if (*maxlen == 0) if (buffer && *currlen == *maxlen) {
*maxlen = 1024; *maxlen += 1024;
*buffer = OPENSSL_malloc(*maxlen); if (*buffer == NULL) {
if (!*buffer) { *buffer = OPENSSL_malloc(*maxlen);
/* Panic! Can't really do anything sensible. Just return */ if (!*buffer) {
return; /* Panic! Can't really do anything sensible. Just return */
} return;
if (*currlen > 0) { }
assert(*sbuffer != NULL); if (*currlen > 0) {
memcpy(*buffer, *sbuffer, *currlen); assert(*sbuffer != NULL);
} memcpy(*buffer, *sbuffer, *currlen);
*sbuffer = NULL; }
} else { *sbuffer = NULL;
*maxlen += 1024; } else {
*buffer = OPENSSL_realloc(*buffer, *maxlen); *buffer = OPENSSL_realloc(*buffer, *maxlen);
if (!*buffer) { if (!*buffer) {
/* Panic! Can't really do anything sensible. Just return */ /* Panic! Can't really do anything sensible. Just return */
return; return;
}
} }
} }
/* What to do if *buffer is NULL? */
assert(*sbuffer != NULL || *buffer != NULL);
} }
if (*currlen < *maxlen) { if (*currlen < *maxlen) {