From 1c8c855611a3708281fa99e3ac21ff8fa063f91c Mon Sep 17 00:00:00 2001 From: Chris Larsen Date: Mon, 10 Apr 2023 21:34:22 -0700 Subject: [PATCH] Fix for #2269 and #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. --- src/tsd/HttpQuery.java | 13 +++++++++++-- test/tsd/TestHttpQuery.java | 23 +++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/tsd/HttpQuery.java b/src/tsd/HttpQuery.java index 2a7d81fa44..9cc93167dd 100644 --- 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,6 +374,10 @@ 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", "
" @@ -380,7 +385,7 @@ public void internalError(final Exception cause) { + "Oops, sorry but your request failed due to a" + " server error.

" + "Please try again in 30 seconds.
"
-                         + pretty_exc
+                         + response
                          + "
")); } } @@ -420,6 +425,10 @@ 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", "
" @@ -427,7 +436,7 @@ public void badRequest(final BadRequestException exception) { + "Sorry but your request was rejected as being" + " invalid.

" + "The reason provided was:
" - + exception.getMessage() + + response + "
")); } } diff --git a/test/tsd/TestHttpQuery.java b/test/tsd/TestHttpQuery.java index 5660ce0461..1efa626145 100644 --- 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("")); + + assertEquals(HttpResponseStatus.INTERNAL_SERVER_ERROR, + query.response().getStatus()); + assertTrue(query.response().getContent().toString(Charset.forName("UTF-8")).contains( + "<script>alert(document.cookie)</script>" + )); + } @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("")); + + assertEquals(HttpResponseStatus.BAD_REQUEST, query.response().getStatus()); + assertTrue(query.response().getContent().toString(Charset.forName("UTF-8")).contains( + "The reason provided was:
<script>alert(document.cookie)</script>" + )); + } @Test public void badRequestDeprecatedJSON() {