Bug 146494 – REGRESSION (185319): Reproducible crash in WebHistoryItem launching FluidApp

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

[Brady Eidson]

REGRESSION (185319): Reproducible crash in WebHistoryItem launching FluidApp

Steps to reproduce:
1 - Download FluidApp from http://fluidapp.com
2 - Create an app from a website (I used webkit.org)
3 - Launch that WebApp

Crash:
1   0x1034112e0 WTFCrash
2   0x104dbe7d9 WTF::CrashOnOverflow::crash()
3   0x104dbe7a9 WTF::CrashOnOverflow::overflowed()
4   0x104dbf471 WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul>::at(unsigned long)
5   0x104e51edd WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul>::operator[](unsigned long)
6   0x104eb909b -[WebHistoryItem initFromDictionaryRepresentation:]
7   0x104eb0897 -[WebHistoryPrivate loadHistoryGutsFromURL:savedItemsCount:collectDiscardedItemsInto:error:]
8   0x104eb0a8c -[WebHistoryPrivate loadFromURL:collectDiscardedItemsInto:error:]
9   0x104eb17db -[WebHistory loadFromURL:error:]
10  0x100019c11 -[FUHistoryController setUpHistory]
11  0x1000198ca -[FUHistoryController init]
12  0x100019807 +[FUHistoryController instance]
13  0x10000e293 -[FUApplication finishLaunching]
14  0x7fff8b010a21 -[NSApplication run]
15  0x7fff8af8d354 NSApplicationMain
16  0x1000017c4 start

The code in question:
        auto redirectURLsVector = std::make_unique<Vector<String>>();

        for (NSUInteger i = 0; i < size; ++i) {
            id redirectURL = [redirectURLs objectAtIndex:i];
            if (![redirectURL isKindOfClass:[NSString class]])
                continue;

            (*redirectURLsVector)[i] = (NSString *)redirectURL;
        }

We create a Vector of size 0 then immediately reference the first element.

That's obviously wrong.

In radar as rdar://problem/21598293

[Brady Eidson]

Created attachment 255899 [details]
Patch v1

[Brady Eidson]

Was introduced in http://trac.webkit.org/changeset/185319

[Darin Adler]

Comment on attachment 255899 [details]
Patch v1

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

> Source/WebKit/mac/History/WebHistoryItem.mm:363
>          NSUInteger size = [redirectURLs count];

After making the thing below a modern Objective-C for loop this can be called right in the reserveInitialCapacity call below.

> Source/WebKit/mac/History/WebHistoryItem.mm:365
> +        redirectURLsVector->reserveCapacity(size);

This should be reserveInitialCapacity.

> Source/WebKit/mac/History/WebHistoryItem.mm:368
>          for (NSUInteger i = 0; i < size; ++i) {
>              id redirectURL = [redirectURLs objectAtIndex:i];

This should be a modern Objective-C for loop:

    for (id redirectURL in redirectURLs) {

> Source/WebKit/mac/History/WebHistoryItem.mm:372
> +            redirectURLsVector->append((NSString *)redirectURL);

This should be uncheckedAppend.

[Brady Eidson]

https://trac.webkit.org/changeset/186179