xss on template engines fix #476

jooby-project · Sep 14, 2016 · a30b327 · a30b327
1 parent 875e718
commit a30b327
Show file tree

Hide file tree

Showing 27 changed files with 404 additions and 47 deletions.
diff --git a/coverage-report/src/test/java/org/jooby/ftl/Issue476FtlXss.java b/coverage-report/src/test/java/org/jooby/ftl/Issue476FtlXss.java
@@ -0,0 +1,29 @@
+package org.jooby.ftl;
+
+import org.jooby.Results;
+import org.jooby.test.ServerFeature;
+import org.jooby.xss.XSS;
+import org.junit.Test;
+
+public class Issue476FtlXss extends ServerFeature {
+
+  {
+    use(new XSS());
+
+    use(new Ftl());
+
+    get("/", req -> Results.html("org/jooby/ftl/xss").put("input", "<script>alert('xss');</script>"));
+  }
+
+  @Test
+  public void xssFn() throws Exception {
+    request()
+        .get("/")
+        .expect("<!DOCTYPE html>\n" +
+            "<html>\n" +
+            "  <body><a href=\"javascript:hello('&#x5C;u003Cscript&#x5C;u003Ealert%28&#x5C;u0027xss&#x5C;u0027%29%3B&#x5C;u003C&#x5C;u002Fscript&#x5C;u003E')\"></a>\n" +
+            "  </body>\n" +
+            "</html>");
+  }
+
+}
diff --git a/coverage-report/src/test/java/org/jooby/hbs/Issue476HbsXss.java b/coverage-report/src/test/java/org/jooby/hbs/Issue476HbsXss.java
@@ -0,0 +1,31 @@
+package org.jooby.hbs;
+
+import org.jooby.Results;
+import org.jooby.test.ServerFeature;
+import org.jooby.xss.XSS;
+import org.junit.Test;
+
+public class Issue476HbsXss extends ServerFeature {
+
+  {
+    use(new XSS());
+
+    use(new Hbs());
+
+    get("/",
+        req -> Results.html("org/jooby/hbs/xss").put("input", "<script>alert('xss');</script>"));
+  }
+
+  @Test
+  public void xssFn() throws Exception {
+    request()
+        .get("/")
+        .expect("<!DOCTYPE html>\n" +
+            "<html>\n" +
+            "  <body><a href=\"javascript:hello('&#x5C;u003Cscript&#x5C;u003Ealert%28&#x5C;u0027xss&#x5C;u0027%29%3B&#x5C;u003C&#x5C;u002Fscript&#x5C;u003E')\"></a>\n"
+            +
+            "  </body>\n" +
+            "</html>");
+  }
+
+}
diff --git a/coverage-report/src/test/java/org/jooby/jade/Issue476JadeXss.java b/coverage-report/src/test/java/org/jooby/jade/Issue476JadeXss.java
@@ -0,0 +1,29 @@
+package org.jooby.jade;
+
+import org.jooby.Results;
+import org.jooby.test.ServerFeature;
+import org.jooby.xss.XSS;
+import org.junit.Test;
+
+public class Issue476JadeXss extends ServerFeature {
+
+  {
+    use(new XSS());
+
+    use(new Jade(".html"));
+
+    get("/",
+        req -> Results.html("org/jooby/jade/xss").put("input", "<script>alert('xss');</script>"));
+  }
+
+  @Test
+  public void xssFn() throws Exception {
+    request()
+        .get("/")
+        .expect("<!DOCTYPE html>\n" +
+            "<html>\n" +
+            "  <body><a href=\"javascript:hello('&amp;#x5C;u003Cscript&amp;#x5C;u003Ealert%28&amp;#x5C;u0027xss&amp;#x5C;u0027%29%3B&amp;#x5C;u003C&amp;#x5C;u002Fscript&amp;#x5C;u003E')\"></a></body>\n" +
+            "</html>");
+  }
+
+}
diff --git a/coverage-report/src/test/java/org/jooby/pebble/Issue476PebbleXss.java b/coverage-report/src/test/java/org/jooby/pebble/Issue476PebbleXss.java
@@ -0,0 +1,29 @@
+package org.jooby.pebble;
+
+import org.jooby.Results;
+import org.jooby.test.ServerFeature;
+import org.jooby.xss.XSS;
+import org.junit.Test;
+
+public class Issue476PebbleXss extends ServerFeature {
+
+  {
+    use(new XSS());
+
+    use(new Pebble());
+
+    get("/", req -> Results.html("org/jooby/pebble/xss").put("input", "<script>alert('xss');</script>"));
+  }
+
+  @Test
+  public void xssFn() throws Exception {
+    request()
+        .get("/")
+        .expect("<!DOCTYPE html>\n" +
+            "<html>\n" +
+            "  <body><a href=\"javascript:hello('&amp;#x5C;u003Cscript&amp;#x5C;u003Ealert%28&amp;#x5C;u0027xss&amp;#x5C;u0027%29%3B&amp;#x5C;u003C&amp;#x5C;u002Fscript&amp;#x5C;u003E')\"></a>\n" +
+            "  </body>\n" +
+            "</html>");
+  }
+
+}
diff --git a/coverage-report/src/test/resources/org/jooby/ftl/xss.html b/coverage-report/src/test/resources/org/jooby/ftl/xss.html
@@ -0,0 +1,5 @@
+<!DOCTYPE html>
+<html>
+  <body><a href="javascript:hello('${xss(input, "js", "uri", "html")}')"></a>
+  </body>
+</html>
diff --git a/coverage-report/src/test/resources/org/jooby/hbs/xss.html b/coverage-report/src/test/resources/org/jooby/hbs/xss.html
@@ -0,0 +1,5 @@
+<!DOCTYPE html>
+<html>
+  <body><a href="javascript:hello('{{xss input 'js' 'uri' 'html'}}')"></a>
+  </body>
+</html>
diff --git a/coverage-report/src/test/resources/org/jooby/jade/xss.html.jade b/coverage-report/src/test/resources/org/jooby/jade/xss.html.jade
@@ -0,0 +1,4 @@
+doctype html
+html
+  body
+    a(href= "javascript:hello('" + xss.apply(input, "js", "uri", "html") + "')")
diff --git a/coverage-report/src/test/resources/org/jooby/pebble/xss.html b/coverage-report/src/test/resources/org/jooby/pebble/xss.html
@@ -0,0 +1,5 @@
+<!DOCTYPE html>
+<html>
+  <body><a href="javascript:hello('{{xss (input, 'js', 'uri', 'html')}}')"></a>
+  </body>
+</html>
diff --git a/jooby-csl/src/main/java/org/jooby/xss/XSS.java b/jooby-csl/src/main/java/org/jooby/xss/XSS.java
@@ -57,7 +57,7 @@
  * If you want to learn more about nested context and why they are important have a look at this
  * <a href=
  * "http://security.coverity.com/document/2013/Mar/fixing-xss-a-practical-guide-for-developers.html">nice
- * guide</code> from
+ * guide</a> from
  * <a href="https://github.com/coverity/coverity-security-library">coverity-security-library</a>.
  * </p>
  *

diff --git a/jooby-ftl/src/main/java/org/jooby/ftl/Ftl.java b/jooby-ftl/src/main/java/org/jooby/ftl/Ftl.java
@@ -28,6 +28,7 @@
 import org.jooby.Renderer;
 import org.jooby.internal.ftl.Engine;
 import org.jooby.internal.ftl.GuavaCacheStorage;
+import org.jooby.internal.ftl.XssDirective;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -194,7 +195,7 @@ public void configure(final Env env, final Config config, final Binder binder) {
 
       binder.bind(Configuration.class).toInstance(freemarker);
 
-      Engine engine = new Engine(freemarker, suffix);
+      Engine engine = new Engine(freemarker, suffix, new XssDirective(env));
 
       Multibinder.newSetBinder(binder, Renderer.class)
           .addBinding().toInstance(engine);

diff --git a/jooby-ftl/src/main/java/org/jooby/internal/ftl/Engine.java b/jooby-ftl/src/main/java/org/jooby/internal/ftl/Engine.java
@@ -42,9 +42,12 @@ public class Engine implements View.Engine {
 
   private String suffix;
 
-  public Engine(final Configuration freemarker, final String suffix) {
+  private XssDirective xss;
+
+  public Engine(final Configuration freemarker, final String suffix, final XssDirective xss) {
     this.freemarker = requireNonNull(freemarker, "Freemarker config is required.");
     this.suffix = suffix;
+    this.xss = xss;
   }
 
   @Override
@@ -57,6 +60,7 @@ public void render(final View view, final Renderer.Context ctx) throws Exception
 
     hash.put("_vname", view.name());
     hash.put("_vpath", template.getName());
+    hash.put("xss", xss);
 
     // locals
     hash.putAll(ctx.locals());

diff --git a/jooby-ftl/src/main/java/org/jooby/internal/ftl/XssDirective.java b/jooby-ftl/src/main/java/org/jooby/internal/ftl/XssDirective.java
@@ -0,0 +1,32 @@
+package org.jooby.internal.ftl;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+import org.jooby.Env;
+
+import freemarker.template.TemplateMethodModelEx;
+import freemarker.template.TemplateModelException;
+import freemarker.template.TemplateScalarModel;
+import javaslang.control.Try;
+
+public class XssDirective implements TemplateMethodModelEx {
+
+  private Env env;
+
+  public XssDirective(final Env env) {
+    this.env = env;
+  }
+
+  @SuppressWarnings({"rawtypes", "unchecked" })
+  @Override
+  public Object exec(final List arguments) throws TemplateModelException {
+    List<String> args = (List<String>) arguments.stream()
+        .map(it -> Try.of(() -> ((TemplateScalarModel) it).getAsString()).get())
+        .collect(Collectors.toList());
+    String[] xss = args.subList(1, args.size())
+        .toArray(new String[arguments.size() - 1]);
+    return env.xss(xss).apply(args.get(0));
+  }
+
+}