Bug 43203 – WebBackForwardList::back/ForwardListWithLimit() crashes if passed a limit larger than max int

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[John Sullivan]

WebBackForwardList::backListWithLimit() and forwardListWithLimit() crash if the limit parameter, an unsigned value, is larger than numeric_limits<int>::max().

The crash occurs here, with i == 0 but an empty m_entries:

        WebBackForwardListItem* item = m_entries[i].get();


The crash occurs due to this incorrect logic:

    unsigned size = std::min(backListCount(), static_cast<unsigned>(limit));
    if (!size)
        return ImmutableArray::create();

This is attempting to return early for the empty case, but casting the unsigned limit to an int can make it negative, and thus size is negative, and thus the test for !size fails.

I've got a fix that I'll send out after lunch, if nobody beats me to it.

[John Sullivan]

Created attachment 62989 [details]
Patch to cast to unsigned rather than int, to avoid wrapping

[Adam Roben (:aroben)]

Comment on attachment 62989 [details]
Patch to cast to unsigned rather than int, to avoid wrapping

> -    unsigned size = std::min(backListCount(), static_cast<int>(limit));
> +    unsigned size = std::min(static_cast<unsigned>(backListCount()), limit);

Why does backListCount return an int? Seems like it should return unsigned.

[John Sullivan]

I agree that backForwardCount() should not return an int. Probably all of these functions should deal with size_t's. But I didn't want to get into that territory for this fix.

Checked in as http://trac.webkit.org/changeset/64306

[John Sullivan]

I filed a bug about the inconsistent use of types in this area: <https://bugs.webkit.org/show_bug.cgi?id=43214>