Bug 176232 – transformCanLikelyUseFastPath() can read off the end of a string

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Simon Fraser (smfr)]

transformCanLikelyUseFastPath() can read off the end of a string

[Simon Fraser (smfr)]

Created attachment 319617 [details]
Patch

[Simon Fraser (smfr)]

rdar://problem/33851237

[WebKit Commit Bot]

Comment on attachment 319617 [details]
Patch

Clearing flags on attachment: 319617

Committed r221488: <http://trac.webkit.org/changeset/221488>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 319617 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        Code added in r220382 could read one byte past the end of the string when looking for the 'z'
> +        of a rotateZ() function. The code was actually incorrect, testing for the 'z at i+6 after
> +        already incrementing i by 6. This patch makes the code correctly detect rotateZ().

Is there a test we can write that would have failed before because of the bad "z" check?

> Source/WebCore/css/parser/CSSParserFastPaths.cpp:1271
> -            if (toASCIILower(chars[i + 6]) == 'z')
> +            if (toASCIILower(chars[i]) == 'z')

Not that this tiny bit of efficiency matters, but all of these can be written more efficiently:

    if (isASCIIAlphaCaselessEqual(chars[i], 'z'))

Same applies for all the other checks above.

[Simon Fraser (smfr)]

(In reply to Darin Adler from comment #5)
> Comment on attachment 319617 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=319617&action=review
> 
> > Source/WebCore/ChangeLog:11
> > +        Code added in r220382 could read one byte past the end of the string when looking for the 'z'
> > +        of a rotateZ() function. The code was actually incorrect, testing for the 'z at i+6 after
> > +        already incrementing i by 6. This patch makes the code correctly detect rotateZ().
> 
> Is there a test we can write that would have failed before because of the
> bad "z" check?

No, becuasse we'd just fail the fast parsing and fall back to normal parsing (which is not detectable).

> > Source/WebCore/css/parser/CSSParserFastPaths.cpp:1271
> > -            if (toASCIILower(chars[i + 6]) == 'z')
> > +            if (toASCIILower(chars[i]) == 'z')
> 
> Not that this tiny bit of efficiency matters, but all of these can be
> written more efficiently:
> 
>     if (isASCIIAlphaCaselessEqual(chars[i], 'z'))
> 
> Same applies for all the other checks above.

Good to know, will fix.