diff options
author | Richard Levitte <levitte@openssl.org> | 2019-04-15 13:15:55 +0200 |
---|---|---|
committer | Richard Levitte <levitte@openssl.org> | 2019-04-18 19:21:52 +0200 |
commit | 18111b130abc0f53b41abbbf82b27d7232ec99f2 (patch) | |
tree | 7d4e368625c1a013b1f970b481bda241c4b5dbd6 /apps | |
parent | 2456ae5763dc4b036b3b4cdb9b98de5d46dd221f (diff) |
asn1parse: avoid double free
|str| was used for multiple conflicting purposes. When using
'-strictpem', it's used to uniquely hold a reference to the loaded
payload. However, when using '-strparse', |str| was re-used to hold
the position from where to start parsing.
So when '-strparse' and '-strictpem' are were together, |str| ended up
pointing into data pointed at by |at|, and was yet being freed, with
the result that the payload it held a reference to became a memory
leak, and there was a double free conflict when both |str| and |at|
were being freed.
The situation is resolved by always having |buf| hold the pointer to
the file data, and always and only use |str| to hold the position to
start parsing from. Now, we only need to free |buf| properly and not
|str|.
Fixes #8752
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from https://github.com/openssl/openssl/pull/8753)
(cherry picked from commit 4f29f3a29b8b416a501c7166dbbca5284b198f81)
Diffstat (limited to 'apps')
-rw-r--r-- | apps/asn1pars.c | 12 |
1 files changed, 5 insertions, 7 deletions
diff --git a/apps/asn1pars.c b/apps/asn1pars.c index 62c70b9cc4..c9a843a363 100644 --- a/apps/asn1pars.c +++ b/apps/asn1pars.c @@ -170,17 +170,17 @@ int asn1parse_main(int argc, char **argv) if (derfile && (derout = bio_open_default(derfile, 'w', FORMAT_ASN1)) == NULL) goto end; + if ((buf = BUF_MEM_new()) == NULL) + goto end; if (strictpem) { - if (PEM_read_bio(in, &name, &header, &str, &num) != - 1) { + if (PEM_read_bio(in, &name, &header, &str, &num) != 1) { BIO_printf(bio_err, "Error reading PEM file\n"); ERR_print_errors(bio_err); goto end; } + buf->data = (char *)str; + buf->length = buf->max = num; } else { - - if ((buf = BUF_MEM_new()) == NULL) - goto end; if (!BUF_MEM_grow(buf, BUFSIZ * 8)) goto end; /* Pre-allocate :-) */ @@ -303,8 +303,6 @@ int asn1parse_main(int argc, char **argv) BUF_MEM_free(buf); OPENSSL_free(name); OPENSSL_free(header); - if (strictpem) - OPENSSL_free(str); ASN1_TYPE_free(at); sk_OPENSSL_STRING_free(osk); return ret; |