[webkit-reviews] review denied: [Bug 26918] XSSAuditor should prevent injection of HTML Base tag : [Attachment 32348] Patch with tests

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jul 7 13:10:01 PDT 2009


Adam Barth <abarth at webkit.org> has denied Daniel Bates <dbates at berkeley.edu>'s
request for review:
Bug 26918: XSSAuditor should prevent injection of HTML Base tag
https://bugs.webkit.org/show_bug.cgi?id=26918

Attachment 32348: Patch with tests
https://bugs.webkit.org/attachment.cgi?id=32348&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
This looks pretty good.  Only one detail to change. 

> +	   m_hrefAttrValue =
StringImpl::createStrippingNullCharacters(attr->value().characters(),
attr->value().length());

It doesn't seem right to do the null stripping here.  m_hrefAttrValue should
hold the value of the href attr, nulls and all.

> +bool XSSAuditor::canSetBaseElementURL(const String& url) const

We can either do the null stripping here or find a way to tell findInRequest
not to strip nulls.

Do we have this same null issue with <script src="..."> or canLoadObject?  If
so, we should fix that in a separate bug.


More information about the webkit-reviews mailing list