diff --git a/src/tsd/GraphHandler.java b/src/tsd/GraphHandler.java index bf321e856..2af125ede 100644 --- a/src/tsd/GraphHandler.java +++ b/src/tsd/GraphHandler.java @@ -40,15 +40,17 @@ import com.google.common.base.Strings; import com.google.common.collect.Sets; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - +import net.opentsdb.core.*; import net.opentsdb.core.Const; import net.opentsdb.core.DataPoint; import net.opentsdb.core.DataPoints; import net.opentsdb.core.Query; import net.opentsdb.core.TSDB; import net.opentsdb.core.TSQuery; +import net.opentsdb.core.Tags; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import net.opentsdb.graph.Plot; import net.opentsdb.meta.Annotation; import net.opentsdb.stats.Histogram; @@ -667,6 +669,7 @@ static void setPlotDimensions(final HttpQuery query, final Plot plot) { String wxh = query.getQueryStringParam("wxh"); if (wxh != null && !wxh.isEmpty()) { wxh = URLDecoder.decode(wxh.trim()); + validateString("wxh", wxh); if (!WXH_VALIDATOR.matcher(wxh).find()) { throw new IllegalArgumentException("'wxh' was invalid. " + "Must satisfy the pattern " + WXH_VALIDATOR.toString()); @@ -744,6 +747,7 @@ static void setPlotParams(final HttpQuery query, final Plot plot) { final Map> querystring = query.getQueryString(); String value; if ((value = popParam(querystring, "yrange")) != null) { + validateString("yrange", value, "[:]"); if (!RANGE_VALIDATOR.matcher(value).find()) { throw new BadRequestException("'yrange' was invalid. " + "Must be in the format [min:max]."); @@ -751,6 +755,7 @@ static void setPlotParams(final HttpQuery query, final Plot plot) { params.put("yrange", value); } if ((value = popParam(querystring, "y2range")) != null) { + validateString("y2range", value, "[:]"); if (!RANGE_VALIDATOR.matcher(value).find()) { throw new BadRequestException("'y2range' was invalid. " + "Must be in the format [min:max]."); @@ -758,6 +763,7 @@ static void setPlotParams(final HttpQuery query, final Plot plot) { params.put("y2range", value); } if ((value = popParam(querystring, "ylabel")) != null) { + validateString("ylabel", value, " "); if (!LABEL_VALIDATOR.matcher(value).find()) { throw new BadRequestException("'ylabel' was invalid. Must " + "satisfy the pattern " + LABEL_VALIDATOR.toString()); @@ -765,6 +771,7 @@ static void setPlotParams(final HttpQuery query, final Plot plot) { params.put("ylabel", stringify(value)); } if ((value = popParam(querystring, "y2label")) != null) { + validateString("y2label", value, " "); if (!LABEL_VALIDATOR.matcher(value).find()) { throw new BadRequestException("'y2label' was invalid. Must " + "satisfy the pattern " + LABEL_VALIDATOR.toString()); @@ -772,6 +779,7 @@ static void setPlotParams(final HttpQuery query, final Plot plot) { params.put("y2label", stringify(value)); } if ((value = popParam(querystring, "yformat")) != null) { + validateString("yformat", value, "% "); if (!FORMAT_VALIDATOR.matcher(value).find()) { throw new BadRequestException("'yformat' was invalid. Must " + "satisfy the pattern " + FORMAT_VALIDATOR.toString()); @@ -779,6 +787,7 @@ static void setPlotParams(final HttpQuery query, final Plot plot) { params.put("format y", stringify(value)); } if ((value = popParam(querystring, "y2format")) != null) { + validateString("y2format", value, "% "); if (!FORMAT_VALIDATOR.matcher(value).find()) { throw new BadRequestException("'y2format' was invalid. Must " + "satisfy the pattern " + FORMAT_VALIDATOR.toString()); @@ -786,6 +795,7 @@ static void setPlotParams(final HttpQuery query, final Plot plot) { params.put("format y2", stringify(value)); } if ((value = popParam(querystring, "xformat")) != null) { + validateString("xformat", value, "% "); if (!FORMAT_VALIDATOR.matcher(value).find()) { throw new BadRequestException("'xformat' was invalid. Must " + "satisfy the pattern " + FORMAT_VALIDATOR.toString()); @@ -799,6 +809,7 @@ static void setPlotParams(final HttpQuery query, final Plot plot) { params.put("logscale y2", ""); } if ((value = popParam(querystring, "key")) != null) { + validateString("key", value); if (!KEY_VALIDATOR.matcher(value).find()) { throw new BadRequestException("'key' was invalid. Must " + "satisfy the pattern " + KEY_VALIDATOR.toString()); @@ -806,6 +817,7 @@ static void setPlotParams(final HttpQuery query, final Plot plot) { params.put("key", value); } if ((value = popParam(querystring, "title")) != null) { + validateString("title", value, " "); if (!LABEL_VALIDATOR.matcher(value).find()) { throw new BadRequestException("'title' was invalid. Must " + "satisfy the pattern " + LABEL_VALIDATOR.toString()); @@ -813,6 +825,7 @@ static void setPlotParams(final HttpQuery query, final Plot plot) { params.put("title", stringify(value)); } if ((value = popParam(querystring, "bgcolor")) != null) { + validateString("bgcolor", value); if (!COLOR_VALIDATOR.matcher(value).find()) { throw new BadRequestException("'bgcolor' was invalid. Must " + "be a hex value e.g. 'xFFFFFF'"); @@ -820,6 +833,7 @@ static void setPlotParams(final HttpQuery query, final Plot plot) { params.put("bgcolor", value); } if ((value = popParam(querystring, "fgcolor")) != null) { + validateString("fgcolor", value); if (!COLOR_VALIDATOR.matcher(value).find()) { throw new BadRequestException("'fgcolor' was invalid. Must " + "be a hex value e.g. 'xFFFFFF'"); @@ -827,6 +841,7 @@ static void setPlotParams(final HttpQuery query, final Plot plot) { params.put("fgcolor", value); } if ((value = popParam(querystring, "smooth")) != null) { + validateString("smooth", value); if (!SMOOTH_VALIDATOR.matcher(value).find()) { throw new BadRequestException("'smooth' was invalid. Must " + "satisfy the pattern " + SMOOTH_VALIDATOR.toString()); @@ -834,6 +849,7 @@ static void setPlotParams(final HttpQuery query, final Plot plot) { params.put("smooth", value); } if ((value = popParam(querystring, "style")) != null) { + validateString("style", value); if (!STYLE_VALIDATOR.matcher(value).find()) { throw new BadRequestException("'style' was invalid. Must " + "satisfy the pattern " + STYLE_VALIDATOR.toString()); @@ -1071,4 +1087,26 @@ static void logError(final HttpQuery query, final String msg, LOG.error(query.channel().toString() + ' ' + msg, e); } + static void validateString(final String what, final String s) { + validateString(what, s, ""); + } + + public static void validateString(final String what, final String s, String specials) { + if (s == null) { + throw new BadRequestException("Invalid " + what + ": null"); + } else if ("".equals(s)) { + throw new BadRequestException("Invalid " + what + ": empty string"); + } + final int n = s.length(); + for (int i = 0; i < n; i++) { + final char c = s.charAt(i); + if (!(('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z') + || ('0' <= c && c <= '9') || c == '-' || c == '_' || c == '.' + || c == '/' || Character.isLetter(c) || specials.indexOf(c) != -1)) { + throw new BadRequestException("Invalid " + what + + " (\"" + s + "\"): illegal character: " + c); + } + } + } + } diff --git a/test/tsd/TestGraphHandler.java b/test/tsd/TestGraphHandler.java index d2fbca43d..d2600f8df 100644 --- a/test/tsd/TestGraphHandler.java +++ b/test/tsd/TestGraphHandler.java @@ -97,6 +97,7 @@ public void setYRangeParams() throws Exception { assertPlotParam("yrange", "[-10.1e-5:]"); assertPlotParam("yrange", "[-10.1e-5:-10.1e-6]"); assertInvalidPlotParam("yrange", "[33:system('touch /tmp/poc.txt')]"); + assertInvalidPlotParam("y2range", "[42:%0a[33:system('touch /tmp/poc.txt')]"); } @Test @@ -109,7 +110,8 @@ public void setKeyParams() throws Exception { assertPlotParam("key", "horiz"); assertPlotParam("key", "box"); assertPlotParam("key", "bottom"); - assertInvalidPlotParam("yrange", "out%20right%20top%0aset%20yrange%20[33:system(%20"); + assertInvalidPlotParam("key", "out%20right%20top%0aset%20yrange%20[33:system(%20"); + assertInvalidPlotParam("key", "%3Bsystem%20%22cat%20/home/ubuntuvm/secret.txt%20%3E/tmp/secret.txt%22%20%22"); } @Test @@ -118,16 +120,23 @@ public void setStyleParams() throws Exception { assertPlotParam("style", "points"); assertPlotParam("style", "circles"); assertPlotParam("style", "dots"); - assertInvalidPlotParam("style", "dots%20[33:system(%20"); + assertInvalidPlotParam("style", "dots%20%0a[33:system(%20"); + assertInvalidPlotParam("style", "%3Bsystem%20%22cat%20/home/ubuntuvm/secret.txt%20%3E/tmp/secret.txt%22%20%22\""); } @Test public void setLabelParams() throws Exception { assertPlotParam("ylabel", "This is good"); assertPlotParam("ylabel", " and so Is this - _ yay"); - assertInvalidPlotParam("ylabel", "[33:system(%20"); - assertInvalidPlotParam("title", "[33:system(%20"); - assertInvalidPlotParam("y2label", "[33:system(%20"); + assertInvalidPlotParam("ylabel", "system(%20no%0anewlines"); + assertInvalidPlotParam("title", "system(%20no%0anewlines"); + assertInvalidPlotParam("y2label", "system(%20no%0anewlines"); + } + + @Test + public void setWXH() throws Exception { + assertPlotDimension("wxh", "720x640"); + assertInvalidPlotDimension("wxh", "720%0ax640"); } @Test @@ -137,12 +146,14 @@ public void setColorParams() throws Exception { assertPlotParam("bgcolor", "%58DEADBE"); assertInvalidPlotParam("bgcolor", "XDEADBEF"); assertInvalidPlotParam("bgcolor", "%5BDEADBE"); + assertInvalidPlotParam("bgcolor", "xBDE%0AAD"); assertPlotParam("fgcolor", "x000000"); assertPlotParam("fgcolor", "XDEADBE"); assertPlotParam("fgcolor", "%58DEADBE"); assertInvalidPlotParam("fgcolor", "XDEADBEF"); assertInvalidPlotParam("fgcolor", "%5BDEADBE"); + assertInvalidPlotParam("fgcolor", "xBDE%0AAD"); } @Test @@ -160,7 +171,8 @@ public void setSmoothParams() throws Exception { assertPlotParam("smooth", "sbezier"); assertPlotParam("smooth", "unwrap"); assertPlotParam("smooth", "zsort"); - assertInvalidPlotParam("smooth", "[33:system(%20"); + assertInvalidPlotParam("smooth", "bezier%20system(%20"); + assertInvalidPlotParam("smooth", "fnormal%0asystem(%20"); } @Test @@ -172,7 +184,8 @@ public void setFormatParams() throws Exception { assertPlotParam("yformat", "%253.0em%25%25"); assertPlotParam("yformat", "%25.2f seconds"); assertPlotParam("yformat", "%25.0f ms"); - assertInvalidPlotParam("yformat", "%252.[33:system"); + assertInvalidPlotParam("yformat", "%252.system(%20"); + assertInvalidPlotParam("yformat", "%252.%0asystem(%20"); } @Test // If the file doesn't exist, we don't use it, obviously. @@ -344,6 +357,13 @@ private static void assertPlotParam(String param, String value) { GraphHandler.setPlotParams(query, plot); } + private static void assertPlotDimension(String param, String value) { + Plot plot = mock(Plot.class); + HttpQuery query = mock(HttpQuery.class); + when(query.getQueryStringParam(param)).thenReturn(value); + GraphHandler.setPlotParams(query, plot); + } + private static void assertInvalidPlotParam(String param, String value) { Plot plot = mock(Plot.class); HttpQuery query = mock(HttpQuery.class); @@ -357,4 +377,14 @@ private static void assertInvalidPlotParam(String param, String value) { } catch (BadRequestException e) { } } + private static void assertInvalidPlotDimension(String param, String value) { + Plot plot = mock(Plot.class); + HttpQuery query = mock(HttpQuery.class); + when(query.getQueryStringParam(param)).thenReturn(value); + try { + GraphHandler.setPlotDimensions(query, plot); + fail("Expected BadRequestException"); + } catch (BadRequestException e) { } + } + }