The GWT framework at some point introduced SafeHtml as a way to represent XSS-safe HTML values. For all framework methods that accept a plain String value to be interpreted as HTML, they have also added an overload that accepts a SafeHtml object instead. Consequently, use of the plain String-typed parameter should be avoided, since it is more susceptible to accidentally introducing an XSS vulnerability into your application.
While SafeHtml does not strictly guarantee that the code is safe (e.g. someone could still call SafeHtmlUtils.fromTrustedString(String) with some vulnerable client input), it at least forces the caller to carefully consider whether the input at hand can really be trusted or would need to be sanitized first instead of passing it to an API that does not make it obvious that the input will be rendered as actual HTML. This is similar to how java.util.Optional as return value of a method forces the caller to explicitly consider the case that the result may be empty, which is not necessarily the case when given a null result instead.
The following method overloads should be considered “unsafe”:
com.google.gwt.dom.client.Element.setInnerHTML(String)com.google.gwt.user.client.ui.HTML.HTML(String)com.google.gwt.user.client.ui.HTML.HTML(String, Direction)com.google.gwt.user.client.ui.HTML.HTML(String, boolean)com.google.gwt.user.client.ui.HTML.setHTML(String, Direction)com.google.gwt.user.client.ui.HasHTML.setHTML(String)
The last one is an interface method that is implemented by many GWT widgets. So by checking that one, you are getting a whole bag of possible findings.
But there are still more than that, for example com.google.gwt.user.client.ui.Button.Button(String).
It may be difficult to list all here.
Alternatively, you could check for all cases where the parameter of a method or constructor is of type String and has the annotation @IsSafeHtml. This is just a marker for the String-typed parameters that are expected to be used as HTML, before they introduced the actual SafeHtml interface.
Noncompliant:
import com.google.gwt.user.client.ui.Button;
protected Button createButton( String buttonText ) {
return new Button( buttonText ); // NONCOMPLIANT - buttonText is interpreted as HTML and could cause XSS
}
import com.google.gwt.user.client.ui.Button;
import com.google.gwt.safehtml.shared.SafeHtmlUtils;
protected Button createButton( String buttonText ) {
return new Button( SafeHtmlUtils.htmlEscape( buttonText ) ); // NONCOMPLIANT - htmlEscape still produces a String, which for the sake of this rule would still use the "wrong" Button constructor overload.
}
Compliant:
Use the methods that produce SafeHtml from the given String.
import com.google.gwt.user.client.ui.Button;
import com.google.gwt.safehtml.shared.SafeHtmlUtils;
protected Button createButton( String buttonText ) {
return new Button( SafeHtmlUtils.fromString( buttonText ) ); // COMPLIANT - buttonText is HTML-escaped
}
import com.google.gwt.user.client.ui.Button;
import com.google.gwt.safehtml.shared.SimpleHtmlSanitizer;
protected Button createButton( String buttonText ) {
return new Button( SimpleHtmlSanitizer.sanitizeHtml( buttonText ) ); // COMPLIANT - buttonText is HTML-escaped in a lenient way
}
Another advantage of SafeHtml is that we already know that it has been sanitized. When we pass around a plain String instead, we don’t know if a previous caller already sanitized it and whether sanitizing it again would lead to double escaping (which could be the case in the calls above). Passing around SafeHtml avoids this problem.
import com.google.gwt.user.client.ui.Button;
import com.google.gwt.safehtml.shared.SafeHtml;
protected Button createButton( SafeHtml buttonText ) {
return new Button( buttonText ); // COMPLIANT - we already know that the text is sanitized
}
A possible exception could be made for cases where we pass an actual String literal instead of a parameter, since the caller knows it’s content at the call site.
import com.google.gwt.user.client.ui.Button;
protected Button createButton() {
return new Button( "<strong>Press me</strong>" ); // COMPLIANT - HTML from a constant literal is safe.
}
This would be a more convenient form of
import com.google.gwt.user.client.ui.Button;
import com.google.gwt.safehtml.shared.SafeHtmlUtils;
protected Button createButton() {
return new Button( SafeHtmlUtils.fromSafeConstant( "<strong>Press me</strong>" ) ); // Safety check of constant is done in development mode an no-op'd in production mode.
}
or
import com.google.gwt.user.client.ui.Button;
import com.google.gwt.safehtml.shared.SafeHtmlUtils;
protected Button createButton() {
return new Button( SafeHtmlUtils.fromTrustedString( "<strong>Press me</strong>" ) ); // Safety check is always skipped.
}
External references:
- GWT Dev Guide for SafeHtml
Money quote:
A central aspect of these coding guidelines is that developers of GWT applications should not use constructors and methods with
String-typed parameters whose values are interpreted as HTML, and instead use theSafeHtmlequivalent.
Type:
I am torn between “Security Hotspot” and “Vulnerability”. A call to a String-typed overload is not necessarily a security issue, but depending on where the parameter comes from it could be. But “Security Hotspots” don’t show up in the IDE during local development, so you may accidentally introduce the issue and only realize it later in SonarQube. So I’d be in favor of “Vulnerability”.
SonarQube doesn’t have rules that apply exclusively to GWT yet, as far as I know. But the framework has at least had a mention as possible source for taint analysis.