Bug 75578 - REGRESSION (r98912-r99538): Crash in WebKit::WebFrameLoaderClient::didDetectXSS
Summary: REGRESSION (r98912-r99538): Crash in WebKit::WebFrameLoaderClient::didDetectXSS
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac (Intel) OS X 10.7
: P1 Critical
Assignee: Nobody
Keywords: Regression
Depends on:
Reported: 2012-01-04 14:58 PST by Kevin M. Dean
Modified: 2012-01-05 15:04 PST (History)
4 users (show)

See Also:

Crash Log (50.94 KB, text/plain)
2012-01-04 14:58 PST, Kevin M. Dean
no flags Details
proposed fix (1.40 KB, patch)
2012-01-05 11:49 PST, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kevin M. Dean 2012-01-04 14:58:11 PST
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.
Comment 1 Kevin M. Dean 2012-01-04 15:58:15 PST
OK, found the range for when the crash begins.

Back in November:

Comment 2 Adam Barth 2012-01-05 11:29:11 PST
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?
Comment 3 Adam Barth 2012-01-05 11:32:22 PST
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.
Comment 4 Alexey Proskuryakov 2012-01-05 11:49:15 PST
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.
Comment 5 Adam Barth 2012-01-05 11:58:10 PST
Comment on attachment 121307 [details]
proposed fix

Ah!  Bitten by the copy/paste gremlin.
Comment 6 Adam Barth 2012-01-05 12:00:14 PST
> 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.
Comment 7 Kevin M. Dean 2012-01-05 12:17:23 PST
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" 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.
Comment 8 Alexey Proskuryakov 2012-01-05 12:19:06 PST
Comment on attachment 121307 [details]
proposed fix

Oops, forgot to mark for review.
Comment 9 Kevin M. Dean 2012-01-05 12:27:24 PST
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.
Comment 10 Alexey Proskuryakov 2012-01-05 12:34:34 PST
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.
Comment 11 WebKit Review Bot 2012-01-05 15:04:42 PST
Comment on attachment 121307 [details]
proposed fix

Clearing flags on attachment: 121307

Committed r104229: <http://trac.webkit.org/changeset/104229>
Comment 12 WebKit Review Bot 2012-01-05 15:04:47 PST
All reviewed patches have been landed.  Closing bug.