Fix for OpenTSDB#2269 and OpenTSDB#2267 XSS vulnerability.

Escaping the user supplied input when outputing the HTML for the old BadRequest HTML handlers should help. Thanks to the reporters. Fixes CVE-2018-13003.
manolama · Apr 11, 2023 · 1c8c855 · 1c8c855
1 parent 9b62442
commit 1c8c855
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 2 deletions.
diff --git a/src/tsd/HttpQuery.java b/src/tsd/HttpQuery.java
@@ -25,6 +25,7 @@
 import java.util.HashSet;
 import java.util.List;
 
+import com.google.common.html.HtmlEscapers;
 import net.opentsdb.core.Const;
 import net.opentsdb.core.TSDB;
 import net.opentsdb.graph.Plot;
@@ -373,14 +374,18 @@ public void internalError(final Exception cause) {
  buf.append("\"}");
  sendReply(HttpResponseStatus.INTERNAL_SERVER_ERROR, buf);
  } else {
+ String response = "";
+ if (pretty_exc != null) {
+ response = HtmlEscapers.htmlEscaper().escape(pretty_exc);
+ }
  sendReply(HttpResponseStatus.INTERNAL_SERVER_ERROR,
  makePage("Internal Server Error", "Houston, we have a problem",
  "<blockquote>"
  + "<h1>Internal Server Error</h1>"
  + "Oops, sorry but your request failed due to a"
  + " server error.<br/><br/>"
  + "Please try again in 30 seconds.<pre>"
- + pretty_exc
+ + response
  + "</pre></blockquote>"));
  }
  }
@@ -420,14 +425,18 @@ public void badRequest(final BadRequestException exception) {
  buf.append("\"}");
  sendReply(HttpResponseStatus.BAD_REQUEST, buf);
  } else {
+ String response = "";
+ if (exception.getMessage() != null) {
+ response = HtmlEscapers.htmlEscaper().escape(exception.getMessage());
+ }
  sendReply(HttpResponseStatus.BAD_REQUEST,
  makePage("Bad Request", "Looks like it's your fault this time",
  "<blockquote>"
  + "<h1>Bad Request</h1>"
  + "Sorry but your request was rejected as being"
  + " invalid.<br/><br/>"
  + "The reason provided was:<blockquote>"
- + exception.getMessage()
+ + response
  + "</blockquote></blockquote>"));
  }
  }

diff --git a/test/tsd/TestHttpQuery.java b/test/tsd/TestHttpQuery.java
@@ -795,6 +795,18 @@ public void internalErrorDeprecated() {
  query.response().getContent().toString(Charset.forName("UTF-8"))
  .substring(0, 15));
  }
+
+ @Test
+ public void internalErrorDeprecatedHTMLEscaped() {
+ HttpQuery query = NettyMocks.getQuery(tsdb, "");
+ query.internalError(new Exception("<script>alert(document.cookie)</script>"));
+
+ assertEquals(HttpResponseStatus.INTERNAL_SERVER_ERROR,
+ query.response().getStatus());
+ assertTrue(query.response().getContent().toString(Charset.forName("UTF-8")).contains(
+ "&lt;script&gt;alert(document.cookie)&lt;/script&gt;"
+ ));
+ }
 
  @Test
  public void internalErrorDeprecatedJSON() {
@@ -849,6 +861,17 @@ public void badRequestDeprecated() {
  query.response().getContent().toString(Charset.forName("UTF-8"))
  .substring(0, 15));
  }
+
+ @Test
+ public void badRequestDeprecatedHTMLEscaped() {
+ HttpQuery query = NettyMocks.getQuery(tsdb, "/");
+ query.badRequest(new BadRequestException("<script>alert(document.cookie)</script>"));
+
+ assertEquals(HttpResponseStatus.BAD_REQUEST, query.response().getStatus());
+ assertTrue(query.response().getContent().toString(Charset.forName("UTF-8")).contains(
+ "The reason provided was:<blockquote>&lt;script&gt;alert(document.cookie)&lt;/script&gt;"
+ ));
+ }
 
  @Test
  public void badRequestDeprecatedJSON() {