Hi,
I find that several error handling sites may forget to free the resource, which is allocated by function BIO_new_file(). See the following code, at line 436, function BIO_new_file() allocates the resource. However, several followed up error handling sites forget to free the resource that allocated by BIO_new_file(), including the handling actions of function sk_BIO_new_null() (line 444 - line 445), sk_BIO_push() (line 449- line 450). For example, function sk_BIO_new_null() does the handling actions: put the error info, then goto the err block. However, in the err block, there doesnot exist the corresonding resource release action related to BIO_new_file(). This may cause a missing resource release bug about function BIO_new_file().
|
#ifndef OPENSSL_NO_POSIX_IO |
|
next = process_include(include_path, &dirctx, &dirpath); |
|
if (include_path != dirpath) { |
|
/* dirpath will contain include in case of a directory */ |
|
OPENSSL_free(include_path); |
|
} |
|
#else |
|
next = BIO_new_file(include_path, "r"); |
|
OPENSSL_free(include_path); |
|
#endif |
|
|
|
if (next != NULL) { |
|
/* push the currently processing BIO onto stack */ |
|
if (biosk == NULL) { |
|
if ((biosk = sk_BIO_new_null()) == NULL) { |
|
CONFerr(CONF_F_DEF_LOAD_BIO, ERR_R_MALLOC_FAILURE); |
|
goto err; |
|
} |
|
} |
|
if (!sk_BIO_push(biosk, in)) { |
|
CONFerr(CONF_F_DEF_LOAD_BIO, ERR_R_MALLOC_FAILURE); |
|
goto err; |
|
} |
|
/* continue with reading from the included BIO */ |
|
in = next; |
|
} |
err block messages:
|
err: |
|
BUF_MEM_free(buff); |
|
OPENSSL_free(section); |
|
/* |
|
* Since |in| is the first element of the stack and should NOT be freed |
|
* here, we cannot use sk_BIO_pop_free(). Instead, we pop and free one |
|
* BIO at a time, making sure that the last one popped isn't. |
|
*/ |
|
while (sk_BIO_num(biosk) > 0) { |
|
BIO *popped = sk_BIO_pop(biosk); |
|
BIO_vfree(in); |
|
in = popped; |
|
} |
|
sk_BIO_free(biosk); |
|
#ifndef OPENSSL_NO_POSIX_IO |
|
OPENSSL_free(dirpath); |
|
if (dirctx != NULL) |
|
OPENSSL_DIR_end(&dirctx); |
|
#endif |
|
if (line != NULL) |
|
*line = eline; |
|
BIO_snprintf(btmp, sizeof(btmp), "%ld", eline); |
|
ERR_add_error_data(2, "line ", btmp); |
|
if (h != conf->data) { |
|
CONF_free(conf->data); |
|
conf->data = NULL; |
|
} |
|
if (v != NULL) { |
|
OPENSSL_free(v->name); |
|
OPENSSL_free(v->value); |
|
OPENSSL_free(v); |
|
} |
|
return 0; |
===========================================================================
Furthermore, I check the other usages of BIO_new_file() from this project. Foe example, at engines/e_capi.c, as shown in the following code, in the end, the resource allocated by BIO_new_file() is freed by the action BIO_free(out) (line 1089) :
|
out = BIO_new_file(ctx->debug_file, "a+"); |
|
if (out == NULL) { |
|
CAPIerr(CAPI_F_CAPI_VTRACE, CAPI_R_FILE_OPEN_ERROR); |
|
return; |
|
} |
|
BIO_vprintf(out, format, argptr); |
|
BIO_free(out); |
