Bug 79135 - equalIgnoringNullity() only comparing half the bytes for equality
Summary: equalIgnoringNullity() only comparing half the bytes for equality
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Thomas Sepez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-21 11:49 PST by Thomas Sepez
Modified: 2012-02-21 16:19 PST (History)
2 users (show)

See Also:


Attachments
Patch plus change to make test flunk without the patch. (2.53 KB, patch)
2012-02-21 13:06 PST, Thomas Sepez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Sepez 2012-02-21 11:49:58 PST
In WebKit/Source/JavaScriptCore/wtf/text/StringImpl.h:731

template<size_t inlineCapacity>
bool equalIgnoringNullity(const Vector<UChar, inlineCapacity>& a, StringImpl* b)
{
    if (!b)
        return !a.size();
    if (a.size() != b->length())
        return false;
    return !memcmp(a.data(), b->characters(), b->length());
}

Which will memcmp() exactly half the bytes since sizeof UChar is 2.

This only gets called from XSSAuditor, and owing to absence of unit tests, the way to test this is via an XSSAuditor test.
Comment 1 Thomas Sepez 2012-02-21 13:06:29 PST
Created attachment 128029 [details]
Patch plus change to make test flunk without the patch.
Comment 2 Adam Barth 2012-02-21 13:21:06 PST
Did you look at other uses of memcmp in that file to make sure they were ok ?
Comment 3 Thomas Sepez 2012-02-21 13:43:31 PST
Just checked that its the only use in the .h, and in the .cpp, the two uses multiply by the size of the templated chartype.
Comment 4 WebKit Review Bot 2012-02-21 16:19:11 PST
Comment on attachment 128029 [details]
Patch plus change to make test flunk without the patch.

Clearing flags on attachment: 128029

Committed r108412: <http://trac.webkit.org/changeset/108412>
Comment 5 WebKit Review Bot 2012-02-21 16:19:16 PST
All reviewed patches have been landed.  Closing bug.