Bug 75578 – REGRESSION (r98912-r99538): Crash in WebKit::WebFrameLoaderClient::didDetectXSS

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Kevin M. Dean]

Created attachment 121162 [details]
Crash Log

I've noticed with recent versions of the Nightly that occassionally when updated pages in WordPress, the web process crashes causes all the tabs to reload and I'm logged out of WordPress. I haven't yet determined a repeatable test case to narrow down which build it starts with, but it does not crash in Safari 5.1.2.

[Kevin M. Dean]

OK, found the range for when the crash begins.

Back in November:

r98912-r99538

[Adam Barth]

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   ???                           	000000000000000000 0 + 0
1   com.apple.WebKit2             	0x0000000108a3253a WebKit::WebFrameLoaderClient::didDetectXSS(WebCore::KURL const&, bool) + 62
2   com.apple.WebCore             	0x0000000109acaf9e WebCore::XSSAuditor::filterToken(WebCore::HTMLToken&) + 334

I presume this is on the WebKit2 Mac build.  Does this stack mean we're crashing inside the WK2 Mac WebKit layer or that the FrameLoaderClient is null?

[Adam Barth]

If we're crashing inside the WK2 code, it might be helpful to have a WK2 expert take a look.  I think I wrote that code by cargo-cult copy/pasting some other FrameLoaderClient method.

[Alexey Proskuryakov]

Created attachment 121307 [details]
proposed fix

Completely untested.

Adam, is it bad that this is even called when working with WordPress? Perhaps we need a separate bug to investigate that.

[Adam Barth]

Comment on attachment 121307 [details]
proposed fix

Ah!  Bitten by the copy/paste gremlin.

[Adam Barth]

> Adam, is it bad that this is even called when working with WordPress? Perhaps we need a separate bug to investigate that.

Probably a good idea.  It might well be a false positive.

@Kevin: Do you know what steps you're taking to hit this crash in word press?  Even after we fix the crash, we'd like to know because its likely that the XSS filter shouldn't be triggering then.

[Kevin M. Dean]

It may be in combination with a plugin.

I'm using the "Deactivate Visual Editor 0.1" plugin on some pages so the HTML formatting is better preserved when updating complicated content. Basically there's no Visual / HTML option on the edit screen. It just defaults to HTML only.

Just opening the page and clicking update submits the update (and it does update in the database, and it looks like the page starts to load again like it normally does and then it crashes, which then causes the page to refresh to the WordPress login screen.

Even though it's not loading on the Visual Editor deactivated pages, I also have "CKEditor for WordPress 3.6.2.4" installed in case it's affecting things behind the scenes.

I'll check and see if disabling any plugins other than "Deactivate Visual Editor" has an effect.

[Alexey Proskuryakov]

Comment on attachment 121307 [details]
proposed fix

Oops, forgot to mark for review.

[Kevin M. Dean]

Disabling the plugins didn't seem to have an effect on the crash. It just doesn't crash on all page edits, but consistently on at least this one test page I'm using.

[Alexey Proskuryakov]

Comment on attachment 121307 [details]
proposed fix

Kevin, this should fix the crash, but something likely won't work as expected in your workflow, because it will be blocked by XSS Auditor.

Please file a new bug with detailed step by step instructions on how to reproduce.

[WebKit Review Bot]

Comment on attachment 121307 [details]
proposed fix

Clearing flags on attachment: 121307

Committed r104229: <http://trac.webkit.org/changeset/104229>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.