I think this is pretty much ready, except for one thing I just thought
of. Sorry, I should have thought of that earlier :/
In ClippedImageImpl, we're using a SafeUri in the context of a url()
style expression ("background: url({3})"). However, the contract of
SafeUri is currently not tight enough for this to be safe I think:
SafeUri would allow e.g., parentheses, colons and dashes in un-escaped
form in the URL. In a template where the URL appears in a url() css
expression as above, a URL like,
http://harmless.com/foo);background:url(javascript:evil());
would pass UriUtils#sanitizeUri (without getting URI-escaped or
anything), and would result in script execution via CSS injection.
So I think we need to,
- tighten the SafeUri contract so it explicitly specifies which
character set can occur in a SafeUri
- add a sanitizeAndEscapeUri method to UriUtils that both checks for an
allowed scheme and URI/%-escapes characters not in the allowed charset
(or maybe rejects URLs with certain funny characters in them).
I'll have to do some reading up on what characters are OK in the URI in
a CSS context, will get back to you on that.
Thanks again for all your work on this!
--xtof
http://gwt-code-reviews.appspot.com/1380806/diff/16001/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java
File
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java
(right):
http://gwt-code-reviews.appspot.com/1380806/diff/16001/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode127
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:127:
* <li>If the template parameter occurs at the start of a URI-valued
There probably should be an explanation about the distinction between
"start of URL-valued attribute" and "entire URL-valued attribute"?
http://gwt-code-reviews.appspot.com/1380806/diff/16001/user/src/com/google/gwt/user/client/ui/impl/ClippedImageImpl.java
File user/src/com/google/gwt/user/client/ui/impl/ClippedImageImpl.java
(right):
http://gwt-code-reviews.appspot.com/1380806/diff/16001/user/src/com/google/gwt/user/client/ui/impl/ClippedImageImpl.java#newcode46
user/src/com/google/gwt/user/client/ui/impl/ClippedImageImpl.java:46:
UriUtils.fromUntrustedString(GWT.getModuleBaseURL() +
"clear.cache.gif");
I think here it would actually be appropriate to use fromTrustedString
-- the value we're passing through here is fully under the control of
the program (at least I think that GWT.getModuleBaseURL cannot be
modified from the outside).
Actually, if GWT.getModuleBaseURL() + someFile is a common idiom, it
might be worth introducing a utility method in UriUtils that returns an
absolute URL based at moduleBaseURL? If it's not common, it won't be
worth the clutter though.
http://gwt-code-reviews.appspot.com/1380806/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
