Avoiding possible XSS attack through the default error handler. See #1366

Author: imeszaros

jooby/src/main/java/org/jooby/Err.java

@@ -205,7 +205,11 @@
 
 import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+import java.util.function.Supplier;
 
 import com.typesafe.config.Config;
 import org.jooby.funzy.Try;
@@ -287,12 +291,24 @@ public void handle(final Request req, final Response rsp, final Err ex) throws T
       log.error("execution of: {}{} resulted in exception\nRoute:\n{}\n\nStacktrace:",
           req.method(), req.path(), req.route().print(6), ex);
       Config conf = req.require(Config.class);
+      Env env = req.require(Env.class);
       boolean stackstrace = Try.apply(() -> conf.getBoolean("err.stacktrace"))
-          .orElse(req.require(Env.class).name().equals("dev"));
+          .orElse(env.name().equals("dev"));
+
+      Function<Object, String> xssFilter = env.xss("html").compose(Objects::toString);
+      BiFunction<String, Object, String> escaper = (k, v) -> xssFilter.apply(v);
+
+      Supplier<Map<String, Object>> detailsProvider = () -> {
+        Map<String, Object> map = ex.toMap(stackstrace);
+        map.compute("message", escaper);
+        map.compute("reason", escaper);
+        return map;
+      };
+
       rsp.send(
           Results
-              .when(MediaType.html, () -> Results.html(VIEW).put("err", ex.toMap(stackstrace)))
-              .when(MediaType.all, () -> ex.toMap(stackstrace)));
+              .when(MediaType.html, () -> Results.html(VIEW).put("err", detailsProvider.get()))
+              .when(MediaType.all, detailsProvider::get));
     }
 
   }

jooby/src/main/java/org/jooby/internal/HttpHandlerImpl.java

@@ -627,7 +627,7 @@ private static Route[] routes(final Set<Route.Definition> routeDefs, final Strin
           // default /favicon.ico handler:
           rsp.status(Status.NOT_FOUND).end();
         } else {
-          throw new Err(Status.NOT_FOUND, req.path(true));
+          throw new Err(Status.NOT_FOUND, req.path());
         }
       }
     }, method, path, "err", accept));

jooby/src/main/java/org/jooby/internal/RouteImpl.java

@@ -233,7 +233,7 @@ public class RouteImpl implements RouteWithFilter {
   public static RouteWithFilter notFound(final String method, final String path) {
     return new FallbackRoute("404", method, path, MediaType.ALL, (req, rsp, chain) -> {
       if (!rsp.status().isPresent()) {
-        throw new Err(Status.NOT_FOUND, req.path(true));
+        throw new Err(Status.NOT_FOUND, req.path());
       }
     });
   }

jooby/src/test/java/org/jooby/DefaultErrHandlerTest.java

@@ -1,6 +1,8 @@
 package org.jooby;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.escape.Escapers;
+import com.google.common.html.HtmlEscapers;
 import com.typesafe.config.Config;
 import static org.easymock.EasyMock.expect;
 import org.jooby.test.MockUnit;
@@ -66,6 +68,7 @@ private MockUnit.Block handleErr(Throwable ex, boolean stacktrace) {
       expect(conf.getBoolean("err.stacktrace")).andReturn(stacktrace);
       Env env = unit.get(Env.class);
       expect(env.name()).andReturn("dev");
+      expect(env.xss("html")).andReturn(HtmlEscapers.htmlEscaper()::escape);
 
       Request req = unit.get(Request.class);
 
@@ -112,6 +115,37 @@ public void handleWithErrMessage() throws Exception {
             });
   }
 
+  @SuppressWarnings({"unchecked"})
+  @Test
+  public void handleWithHtmlErrMessage() throws Exception {
+    Err ex = new Err(500, "Something something <em>dark</em>");
+
+    StringWriter writer = new StringWriter();
+    ex.printStackTrace(new PrintWriter(writer));
+    String[] stacktrace = writer.toString().replace("\r", "").split("\\n");
+
+    new MockUnit(Request.class, Response.class, Route.class, Env.class, Config.class)
+            .expect(handleErr(ex, true))
+            .run(unit -> {
+
+                      Request req = unit.get(Request.class);
+                      Response rsp = unit.get(Response.class);
+
+                      new Err.DefHandler().handle(req, rsp, ex);
+                    },
+                    unit -> {
+                      Result result = unit.captured(Result.class).iterator().next();
+                      View view = (View) result.ifGet(ImmutableList.of(MediaType.html)).get();
+                      assertEquals("err", view.name());
+                      checkErr(stacktrace, "Server Error(500): Something something &lt;em&gt;dark&lt;/em&gt;",
+                              (Map<String, Object>) view.model()
+                                      .get("err"));
+
+                      Object hash = result.ifGet(MediaType.ALL).get();
+                      assertEquals(4, ((Map<String, Object>) hash).size());
+                    });
+  }
+
   private void checkErr(final String[] stacktrace, final String message,
       final Map<String, Object> err) {
     assertEquals(message, err.remove("message"));