Bug 210646 – REGRESSION (r260112): createArchiveList() leaks malloc memory on early returns due to an error

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[David Kilzer (:ddkilzer)]

createArchiveList() leaks malloc memory on early returns due to an error.

This was introduced recently by:

Bug 210456: dictionaryValueOfType() in WebCoreArgumentCodersMac.mm can be replaced with dynamic_cf_cast<>()
<https://webkit.org/b/210456>
<https://trac.webkit.org/r260112>

Found by clang static analyzer running in deep mode.

[Radar WebKit Bug Importer]

<rdar://problem/61928031>

[David Kilzer (:ddkilzer)]

Created attachment 396751 [details]
Patch v1

[Darin Adler]

Comment on attachment 396751 [details]
Patch v1

We should use smart pointers, not raw pointers, so it is harder to make mistakes like this. We have a MallocPtr template that we could use to work with straight malloc/free. There’s a little work needed since by default it works with fastMalloc/fastFree, but it might be worthwhile.

[Darin Adler]

Comment on attachment 396751 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=396751&action=review

> Source/WebKit/Shared/mac/WebCoreArgumentCodersMac.mm:143
> -    if (!extractDictionaryValue(representation, CFSTR("protocolProperties"), protocolProperties))
> -        return false;
> -    if (!extractDictionaryValue(representation, CFSTR("expectedContentLength"), expectedContentLength))
> -        return false;
> -    if (!extractDictionaryValue(representation, CFSTR("mimeType"), mimeType))
> +    if (!extractDictionaryValue(representation, CFSTR("protocolProperties"), protocolProperties)
> +        || !extractDictionaryValue(representation, CFSTR("expectedContentLength"), expectedContentLength)
> +        || !extractDictionaryValue(representation, CFSTR("mimeType"), mimeType)) {
> +        free(*objects);
> +        *objects = nullptr;
> +        *objectCount = 0;
>          return false;
> +    }

Another fix would be to do this checking and extraction before calling malloc. No reason things have to be done in this order

[David Kilzer (:ddkilzer)]

Created attachment 396808 [details]
Patch for landing

[EWS]

Committed r260299: <https://trac.webkit.org/changeset/260299>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 396808 [details].