Skip to content

Commit 1c8c855

Browse files
committed
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.
1 parent 9b62442 commit 1c8c855

File tree

2 files changed

+34
-2
lines changed

2 files changed

+34
-2
lines changed

src/tsd/HttpQuery.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.util.HashSet;
2626
import java.util.List;
2727

28+
import com.google.common.html.HtmlEscapers;
2829
import net.opentsdb.core.Const;
2930
import net.opentsdb.core.TSDB;
3031
import net.opentsdb.graph.Plot;
@@ -373,14 +374,18 @@ public void internalError(final Exception cause) {
373374
buf.append("\"}");
374375
sendReply(HttpResponseStatus.INTERNAL_SERVER_ERROR, buf);
375376
} else {
377+
String response = "";
378+
if (pretty_exc != null) {
379+
response = HtmlEscapers.htmlEscaper().escape(pretty_exc);
380+
}
376381
sendReply(HttpResponseStatus.INTERNAL_SERVER_ERROR,
377382
makePage("Internal Server Error", "Houston, we have a problem",
378383
"<blockquote>"
379384
+ "<h1>Internal Server Error</h1>"
380385
+ "Oops, sorry but your request failed due to a"
381386
+ " server error.<br/><br/>"
382387
+ "Please try again in 30 seconds.<pre>"
383-
+ pretty_exc
388+
+ response
384389
+ "</pre></blockquote>"));
385390
}
386391
}
@@ -420,14 +425,18 @@ public void badRequest(final BadRequestException exception) {
420425
buf.append("\"}");
421426
sendReply(HttpResponseStatus.BAD_REQUEST, buf);
422427
} else {
428+
String response = "";
429+
if (exception.getMessage() != null) {
430+
response = HtmlEscapers.htmlEscaper().escape(exception.getMessage());
431+
}
423432
sendReply(HttpResponseStatus.BAD_REQUEST,
424433
makePage("Bad Request", "Looks like it's your fault this time",
425434
"<blockquote>"
426435
+ "<h1>Bad Request</h1>"
427436
+ "Sorry but your request was rejected as being"
428437
+ " invalid.<br/><br/>"
429438
+ "The reason provided was:<blockquote>"
430-
+ exception.getMessage()
439+
+ response
431440
+ "</blockquote></blockquote>"));
432441
}
433442
}

test/tsd/TestHttpQuery.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -795,6 +795,18 @@ public void internalErrorDeprecated() {
795795
query.response().getContent().toString(Charset.forName("UTF-8"))
796796
.substring(0, 15));
797797
}
798+
799+
@Test
800+
public void internalErrorDeprecatedHTMLEscaped() {
801+
HttpQuery query = NettyMocks.getQuery(tsdb, "");
802+
query.internalError(new Exception("<script>alert(document.cookie)</script>"));
803+
804+
assertEquals(HttpResponseStatus.INTERNAL_SERVER_ERROR,
805+
query.response().getStatus());
806+
assertTrue(query.response().getContent().toString(Charset.forName("UTF-8")).contains(
807+
"&lt;script&gt;alert(document.cookie)&lt;/script&gt;"
808+
));
809+
}
798810

799811
@Test
800812
public void internalErrorDeprecatedJSON() {
@@ -849,6 +861,17 @@ public void badRequestDeprecated() {
849861
query.response().getContent().toString(Charset.forName("UTF-8"))
850862
.substring(0, 15));
851863
}
864+
865+
@Test
866+
public void badRequestDeprecatedHTMLEscaped() {
867+
HttpQuery query = NettyMocks.getQuery(tsdb, "/");
868+
query.badRequest(new BadRequestException("<script>alert(document.cookie)</script>"));
869+
870+
assertEquals(HttpResponseStatus.BAD_REQUEST, query.response().getStatus());
871+
assertTrue(query.response().getContent().toString(Charset.forName("UTF-8")).contains(
872+
"The reason provided was:<blockquote>&lt;script&gt;alert(document.cookie)&lt;/script&gt;"
873+
));
874+
}
852875

853876
@Test
854877
public void badRequestDeprecatedJSON() {

0 commit comments

Comments
 (0)