CVE-2023-36812: Improper Neutralization of Special Elements in Output Used by a Downstream Component ('Injection') (CWE-74)
value: OpenTSDB is a open source, distributed, scalable Time Series Database (TSDB). OpenTSDB is vulnerable to Remote Code Execution vulnerability by writing user-controlled input to Gnuplot configuration file and running Gnuplot with the generated configuration. This issue has been patched in  commit `07c4641471c` and further refined in commit `fa88d3e4b`. These patches are available in the `2.4.2` release. Users are advised to upgrade. User unable to upgrade may disable Gunuplot via the config option`tsd.core.enable_ui = true` and remove the shell files `mygnuplot.bat` and `mygnuplot.sh`.
CWE description: The product constructs all or part of a command, data structure, or record using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify how it is parsed or interpreted when it is sent to a downstream component.
CVSS v3 base score: 9.8
Published: 2023-06-30T23:15Z
Repository: https://github.com/OpenTSDB/opentsdb
Language: Java
Files changed: src/tsd/HttpQuery.java, test/tsd/TestHttpQuery.java, test/tsd/TestQueryRpc.java
Commit date: 2023-04-11 09:53:14 -0700
Commit message: 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.

Diff: diff --git a/src/tsd/HttpQuery.java b/src/tsd/HttpQuery.java
index 2a7d81f..9cc9316 100644
--- a/src/tsd/HttpQuery.java
+++ b/src/tsd/HttpQuery.java
@@ -23,10 +23,11 @@ import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.HashMap;
 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;
 import net.opentsdb.stats.Histogram;
 import net.opentsdb.stats.StatsCollector;
@@ -371,18 +372,22 @@ final class HttpQuery extends AbstractHttpQuery {
       buf.append("{\"err\":\"");
       HttpQuery.escapeJson(pretty_exc, buf);
       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>"));
     }
   }
 
   /**
@@ -418,18 +423,22 @@ final class HttpQuery extends AbstractHttpQuery {
       buf.append("{\"err\":\"");
       HttpQuery.escapeJson(exception.getMessage(), buf);
       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
index 5660ce0..1efa626 100644
--- a/test/tsd/TestHttpQuery.java
+++ b/test/tsd/TestHttpQuery.java
@@ -793,10 +793,22 @@ public final class TestHttpQuery {
     assertEquals(
         "<!DOCTYPE html>", 
         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() {
     HttpQuery query = NettyMocks.getQuery(tsdb, "/?json");
     try {
@@ -847,10 +859,21 @@ public final class TestHttpQuery {
     assertEquals(
         "<!DOCTYPE html>", 
         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() {
     HttpQuery query = NettyMocks.getQuery(tsdb, "/?json");
     try {
diff --git a/test/tsd/TestQueryRpc.java b/test/tsd/TestQueryRpc.java
index f9f855a..c38b3bc 100644
--- a/test/tsd/TestQueryRpc.java
+++ b/test/tsd/TestQueryRpc.java
@@ -516,11 +516,11 @@ public final class TestQueryRpc {
           "\"rateOptions\":{\"counter\":false}}]}");
     rpc.execute(tsdb, query);
     assertEquals(HttpResponseStatus.BAD_REQUEST, query.response().getStatus());
     final String json =
         query.response().getContent().toString(Charset.forName("UTF-8"));
-    assertTrue(json.contains("No such name for 'foo': 'metrics'"));
+    assertTrue(json.contains("No such name for &#39;foo&#39;: &#39;metrics&#39;"));
   }
 
   @Test
   public void executeEmpty() throws Exception {
     final HttpQuery query = NettyMocks.getQuery(tsdb,
@@ -577,11 +577,11 @@ public final class TestQueryRpc {
         "/api/query?start=1h-ago&m=sum:sys.cpu.user");
     rpc.execute(tsdb, query);
     assertEquals(HttpResponseStatus.BAD_REQUEST, query.response().getStatus());
     final String json =
         query.response().getContent().toString(Charset.forName("UTF-8"));
-    assertTrue(json.contains("No such name for 'foo': 'metrics'"));
+    assertTrue(json.contains("No such name for &#39;foo&#39;: &#39;metrics&#39;"));
   }
 
   @Test
   public void executeWithBadDSFill() throws Exception {
     final DataPoints[] datapoints = new DataPoints[1];

Vulnerable code:                          + pretty_exc
                         + exception.getMessage()
    assertTrue(json.contains("No such name for 'foo': 'metrics'"));
    assertTrue(json.contains("No such name for 'foo': 'metrics'"));
Fixed code: import com.google.common.html.HtmlEscapers;
      String response = "";
      if (pretty_exc != null) {
        response = HtmlEscapers.htmlEscaper().escape(pretty_exc);
      }
                         + response
      String response = "";
      if (exception.getMessage() != null) {
        response = HtmlEscapers.htmlEscaper().escape(exception.getMessage());
      }
                         + response

  @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 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;"
    ));
  }
    assertTrue(json.contains("No such name for &#39;foo&#39;: &#39;metrics&#39;"));
    assertTrue(json.contains("No such name for &#39;foo&#39;: &#39;metrics&#39;"));