asn1parse: avoid double free
authorRichard Levitte <levitte@openssl.org>
Mon, 15 Apr 2019 11:15:55 +0000 (13:15 +0200)
committerRichard Levitte <levitte@openssl.org>
Thu, 18 Apr 2019 17:21:52 +0000 (19:21 +0200)
|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)

apps/asn1pars.c

index 62c70b9cc4405e97581316e025241d80e614082b..c9a843a363837ac0536f4c0201765a4419dcaefc 100644 (file)
@@ -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;