A certain enterprise application has severely degraded performance in Safari, and it turned out that this is due to XSSAuditor checks.
The problem occurs after the application submits a form - the result loads really slow, beachballing Safari. This happens because the form is huge (about 170Kbytes). Creating a SuffixTree from it takes a long time. In addition, the tree isn't effectively cached (it's re-created each time you go from external script to inline script to an event listener attribute).
Perhaps one doesn't need to create a SuffixTree if form data is longer than the script being executed? Or maybe there is an even better solution?
Of course, one can disable XSS Auditor with HTTP headers as a temporary workaround.
If we cached the tree better, would that fix the issue?
The memory impact of having more than one copy of the tree doesn't appear negligible. Even one copy of a tree for a 170K form is probably not negligible.
We're seeing other reports of this slowness with internal website.
Having a test case would be helpful here. I can try to create one based on the description in Comment #0. It seems like a reasonable first step is to improve the caching of the suffix tree.
Created attachment 79330 [details]
I can't easily share or even access actual data from the internal web applications, but here is a synthetic test case. Works fast in Firefox, of course.
The proximate cause is that the CachingURLCanonicalizer isn't sufficiently associative. Alternating inline event handlers and inline script flips the decodeEntities bit back and forth, causing the cache to thrash.
A short-term fix is to make the cache more associative. A long-term fix is to move to a new XSS detection architecture, which I've wanted to do for a while...
I'm going to try for the short term fix first.
We talked on IRC. I'm going to try the long-term fix first. I'll post incremental patches blocking this bug.
With the patches up through Bug 53665, this test case is now blazingly fast.
Created attachment 81176 [details]
I'll add the PerformanceTest in a follow-up patch.
Comment on attachment 81176 [details]
Some discussion in IRC:
[7:50pm] dydz: abarth: Some of the expected results have more console messages than before...
[7:50pm] abarth: yeah
[7:50pm] abarth: that's because we block injected "type" attributes
[7:51pm] abarth: for objects now too
[7:51pm] abarth: in XSSAuditor, you could inject an object tag for Gears
[7:51pm] abarth: for example
[7:51pm] abarth: because that doesn't result in a load
[7:51pm] abarth: whereas XSSFilter will block that
[7:52pm] abarth: dydz: i think there's some kind of attack scenario there involving a plugin with a non-standard way of loading stuff
[7:52pm] abarth: dydz: but i don't have a concrete example
Committed r77588: <http://trac.webkit.org/changeset/77588>
> Of course, one can disable XSS Auditor with HTTP headers as a temporary workaround.
Ugh, it turns that one cannot do that with WebKit as released with shipping Safari. XSSAuditor::findInRequest() only checks for X-XSS-Protection header after performing all time consuming operations!
(In reply to comment #15)
> > Of course, one can disable XSS Auditor with HTTP headers as a temporary workaround.
> Ugh, it turns that one cannot do that with WebKit as released with shipping Safari. XSSAuditor::findInRequest() only checks for X-XSS-Protection header after performing all time consuming operations!
That's unfortunate. :(
The new design shouldn't have that problem...