[Webkit-unassigned] [Bug 26918] XSSAuditor should prevent injection of HTML Base tag

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=26918





--- Comment #3 from Adam Barth <abarth at webkit.org>  2009-07-05 23:18:59 PDT ---
(From update of attachment 32284)
Generally looks good, but I have a couple nits and a couple questions:

> +        m_hrefRaw = attr->value();

I might call this something like hrefAttrValue.  "Raw" is kind of a vague term.

> +    KURL baseElementURL(m_frame->document()->url(), url);
> +    if (findInRequest(url) && m_frame->document()->url().baseAsString() != baseElementURL.baseAsString()) {

Seems like you should do the != comparison first because it will be faster than
findInRequest.

> --- LayoutTests/http/tests/security/xssAuditor/base-href-safe2-expected.txt	(revision 0)
> +++ LayoutTests/http/tests/security/xssAuditor/base-href-safe2-expected.txt	(revision 0)
> @@ -0,0 +1,2 @@
> +ALERT: /XSS/

This looks like a failure.  Can we alert something friendlier here?

> +ALERT: This is a safe script.

Like this ^^^^  :)

> +<iframe src="http://localhost:8000/security/xssAuditor/resources/echo-head-base-href.pl?q=<base href='127.0.0.1:8000/security/xssAuditor/resources/base-href/'>">

I think you mean to have a // in the beginning of this href attribute.

> +<iframe src="http://localhost:8000/security/xssAuditor/resources/echo-head-base-href.pl?q=<base href='//127.0.0.1:8000/security/xssAuditor/resources/base-href/'>">

What's the difference between these to test cases?  It looks like the first
isn't actually testing a scheme-relative path.  It's testing a relative path.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.