Skip to content

Commit

Permalink
Tighten up the regexes for Gnuplot URI params per multiple security r…
Browse files Browse the repository at this point in the history 
…eports.

The best way of avoiding RCEs is to disable Gnuplot, but this should help a little.
  • Loading branch information
@manolama
manolama committed Apr 10, 2023
1 parent 9b62442 commit af19333
Show file tree
Hide file tree
Showing 2 changed files with 135 additions and 100 deletions.
14 changes: 7 additions & 7 deletions src/tsd/GraphHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,15 @@ final class GraphHandler implements HttpRpc {

private static final String RANGE_COMPONENT = "\\\"?-?\\d*\\.?(\\d+)?([eE]-?\\d+)?\\\"?";
private static Pattern RANGE_VALIDATOR = Pattern.compile(
"\\["+RANGE_COMPONENT+":"+RANGE_COMPONENT+"]");
private static Pattern LABEL_VALIDATOR = Pattern.compile("[a-zA-z0-9 \\-_]");
"^\\["+RANGE_COMPONENT+":"+RANGE_COMPONENT+"]$");
private static Pattern LABEL_VALIDATOR = Pattern.compile("^[a-zA-z0-9 \\-_]+$");
private static Pattern KEY_VALIDATOR = Pattern.compile(
"(out|left|top|center|right|horiz|box|bottom)?\\s?");
private static Pattern STYLE_VALIDATOR = Pattern.compile("(linespoint|points|circles|dots)");
private static Pattern COLOR_VALIDATOR = Pattern.compile("(x|X)[a-fA-F0-9]{6}");
private static Pattern SMOOTH_VALIDATOR = Pattern.compile("unique|frequency|fnormal|cumulative|cnormal|bins|csplines|acsplines|mcsplines|bezier|sbezier|unwrap|zsort");
"^out|left|top|center|right|horiz|box|bottom$");
private static Pattern STYLE_VALIDATOR = Pattern.compile("^linespoint|points|circles|dots$");
private static Pattern COLOR_VALIDATOR = Pattern.compile("^(x|X)[a-fA-F0-9]{6}$");
private static Pattern SMOOTH_VALIDATOR = Pattern.compile("^unique|frequency|fnormal|cumulative|cnormal|bins|csplines|acsplines|mcsplines|bezier|sbezier|unwrap|zsort$");
// NOTE: This one should be tightened for only time based formatters.
private static Pattern FORMAT_VALIDATOR = Pattern.compile("(%[a-zA-Z])+[:\\/]?\\s?");
private static Pattern FORMAT_VALIDATOR = Pattern.compile("^[%0-9.a-zA-Z \\-]+$");
private static Pattern WXH_VALIDATOR = Pattern.compile("^\\d+x\\d+$");
/** Number of times we had to do all the work up to running Gnuplot. */
private static final AtomicInteger graphs_generated
Expand Down
221 changes: 128 additions & 93 deletions test/tsd/TestGraphHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,102 +67,114 @@ public final class TestGraphHandler {

@Test
public void setYRangeParams() throws Exception {
Plot plot = mock(Plot.class);
HttpQuery query = mock(HttpQuery.class);
Map<String, List<String>> params = Maps.newHashMap();
when(query.getQueryString()).thenReturn(params);
assertPlotParam("yrange","[0:1]");
assertPlotParam("yrange", "[:]");
assertPlotParam("yrange", "[:0]");
assertPlotParam("yrange", "[:42]");
assertPlotParam("yrange", "[:-42]");
assertPlotParam("yrange", "[:0.8]");
assertPlotParam("yrange", "[:-0.8]");
assertPlotParam("yrange", "[:42.4]");
assertPlotParam("yrange", "[:-42.4]");
assertPlotParam("yrange", "[:4e4]");
assertPlotParam("yrange", "[:-4e4]");
assertPlotParam("yrange", "[:4e-4]");
assertPlotParam("yrange", "[:-4e-4]");
assertPlotParam("yrange", "[:4.2e4]");
assertPlotParam("yrange", "[:-4.2e4]");
assertPlotParam("yrange", "[0:]");
assertPlotParam("yrange", "[5:]");
assertPlotParam("yrange", "[-5:]");
assertPlotParam("yrange", "[0.5:]");
assertPlotParam("yrange", "[-0.5:]");
assertPlotParam("yrange", "[10.5:]");
assertPlotParam("yrange", "[-10.5:]");
assertPlotParam("yrange", "[10e5:]");
assertPlotParam("yrange", "[-10e5:]");
assertPlotParam("yrange", "[10e-5:]");
assertPlotParam("yrange", "[-10e-5:]");
assertPlotParam("yrange", "[10.1e-5:]");
assertPlotParam("yrange", "[-10.1e-5:]");
assertPlotParam("yrange", "[-10.1e-5:-10.1e-6]");
assertInvalidPlotParam("yrange", "[33:system('touch /tmp/poc.txt')]");
}

params.put("yrange", Lists.newArrayList("[0:1]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[:]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[:0]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[:42]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[:-42]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[:0.8]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[:-0.8]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[:42.4]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[:-42.4]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[:4e4]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[:-4e4]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[:4e-4]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[:-4e-4]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[:4.2e4]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[:-4.2e4]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[0:]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[5:]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[-5:]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[0.5:]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[-0.5:]"));
GraphHandler.setPlotParams(query, plot);
@Test
public void setKeyParams() throws Exception {
assertPlotParam("key", "out");
assertPlotParam("key", "left");
assertPlotParam("key", "top");
assertPlotParam("key", "center");
assertPlotParam("key", "right");
assertPlotParam("key", "horiz");
assertPlotParam("key", "box");
assertPlotParam("key", "bottom");
assertInvalidPlotParam("yrange", "out%20right%20top%0aset%20yrange%20[33:system(%20");
}

params.put("yrange", Lists.newArrayList("[10.5:]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[-10.5:]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[10e5:]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[-10e5:]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[10e-5:]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[-10e-5:]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[10.1e-5:]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[-10.1e-5:]"));
GraphHandler.setPlotParams(query, plot);

params.put("yrange", Lists.newArrayList("[33:system('touch /tmp/poc.txt')]"));
try {
GraphHandler.setPlotParams(query, plot);
fail("Expected BadRequestException");
} catch (BadRequestException e) { }
@Test
public void setStyleParams() throws Exception {
assertPlotParam("style", "linespoint");
assertPlotParam("style", "points");
assertPlotParam("style", "circles");
assertPlotParam("style", "dots");
assertInvalidPlotParam("style", "dots%20[33:system(%20");
}


@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");
}

@Test
public void setColorParams() throws Exception {
assertPlotParam("bgcolor", "x000000");
assertPlotParam("bgcolor", "XDEADBE");
assertPlotParam("bgcolor", "%58DEADBE");
assertInvalidPlotParam("bgcolor", "XDEADBEF");
assertInvalidPlotParam("bgcolor", "%5BDEADBE");

assertPlotParam("fgcolor", "x000000");
assertPlotParam("fgcolor", "XDEADBE");
assertPlotParam("fgcolor", "%58DEADBE");
assertInvalidPlotParam("fgcolor", "XDEADBEF");
assertInvalidPlotParam("fgcolor", "%5BDEADBE");
}

@Test
public void setSmoothParams() throws Exception {
assertPlotParam("smooth", "unique");
assertPlotParam("smooth", "frequency");
assertPlotParam("smooth", "fnormal");
assertPlotParam("smooth", "cumulative");
assertPlotParam("smooth", "cnormal");
assertPlotParam("smooth", "bins");
assertPlotParam("smooth", "csplines");
assertPlotParam("smooth", "acsplines");
assertPlotParam("smooth", "mcsplines");
assertPlotParam("smooth", "bezier");
assertPlotParam("smooth", "sbezier");
assertPlotParam("smooth", "unwrap");
assertPlotParam("smooth", "zsort");
assertInvalidPlotParam("smooth", "[33:system(%20");
}

@Test
public void setFormatParams() throws Exception {
assertPlotParam("yformat", "%25.2f");
assertPlotParam("y2format", "%25.2f");
assertPlotParam("xformat", "%25.2f");
assertPlotParam("yformat", "%253.0em");
assertPlotParam("yformat", "%253.0em%25%25");
assertPlotParam("yformat", "%25.2f seconds");
assertPlotParam("yformat", "%25.0f ms");
assertInvalidPlotParam("yformat", "%252.[33:system");
}

@Test // If the file doesn't exist, we don't use it, obviously.
public void staleCacheFileDoesntExist() throws Exception {
final File cachedfile = fakeFile("/cache/fake-file");
Expand Down Expand Up @@ -322,4 +334,27 @@ private static File fakeFile(final String path) {
return file;
}

private static void assertPlotParam(String param, String value) {
Plot plot = mock(Plot.class);
HttpQuery query = mock(HttpQuery.class);
Map<String, List<String>> params = Maps.newHashMap();
when(query.getQueryString()).thenReturn(params);

params.put(param, Lists.newArrayList(value));
GraphHandler.setPlotParams(query, plot);
}

private static void assertInvalidPlotParam(String param, String value) {
Plot plot = mock(Plot.class);
HttpQuery query = mock(HttpQuery.class);
Map<String, List<String>> params = Maps.newHashMap();
when(query.getQueryString()).thenReturn(params);

params.put(param, Lists.newArrayList(value));
try {
GraphHandler.setPlotParams(query, plot);
fail("Expected BadRequestException");
} catch (BadRequestException e) { }
}

}

0 comments on commit af19333

Please sign in to comment.