diff --git a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll index 88d74b66db6..57c7af0ca4f 100644 --- a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll +++ b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll @@ -117,7 +117,6 @@ private module Frameworks { private import semmle.code.java.frameworks.Retrofit private import semmle.code.java.frameworks.Stream private import semmle.code.java.frameworks.Strings - private import semmle.code.java.frameworks.Velocity private import semmle.code.java.frameworks.ratpack.Ratpack private import semmle.code.java.frameworks.ratpack.RatpackExec private import semmle.code.java.frameworks.spring.SpringCache diff --git a/java/ql/lib/semmle/code/java/frameworks/Velocity.qll b/java/ql/lib/semmle/code/java/frameworks/Velocity.qll deleted file mode 100644 index 98c6acd60d4..00000000000 --- a/java/ql/lib/semmle/code/java/frameworks/Velocity.qll +++ /dev/null @@ -1,14 +0,0 @@ -/** Definitions related to the Apache Velocity templating library. */ - -import java -private import semmle.code.java.dataflow.ExternalFlow - -private class VelocitySummaryModels extends SummaryModelCsv { - override predicate row(string row) { - row = - [ - "org.apache.velocity.context;AbstractContext;true;put;;;Argument[1];Argument[-1];taint;manual", - "org.apache.velocity.context;AbstractContext;true;internalPut;;;Argument[1];Argument[-1];taint;manual", - ] - } -} diff --git a/java/ql/lib/semmle/code/java/security/TemplateInjection.qll b/java/ql/lib/semmle/code/java/security/TemplateInjection.qll index 2b7f75a7c87..d281945cfd1 100644 --- a/java/ql/lib/semmle/code/java/security/TemplateInjection.qll +++ b/java/ql/lib/semmle/code/java/security/TemplateInjection.qll @@ -89,18 +89,14 @@ private class TemplateInjectionSinkModels extends SinkModelCsv { "com.hubspot.jinjava;Jinjava;true;render;;;Argument[0];ssti;manual", "org.thymeleaf;ITemplateEngine;true;process;;;Argument[0];ssti;manual", "org.thymeleaf;ITemplateEngine;true;processThrottled;;;Argument[0];ssti;manual", - "org.apache.velocity.app;Velocity;true;evaluate;;;Argument[0];ssti;manual", "org.apache.velocity.app;Velocity;true;evaluate;;;Argument[3];ssti;manual", "org.apache.velocity.app;Velocity;true;mergeTemplate;;;Argument[2];ssti;manual", - "org.apache.velocity.app;VelocityEngine;true;evaluate;;;Argument[0];ssti;manual", "org.apache.velocity.app;VelocityEngine;true;evaluate;;;Argument[3];ssti;manual", "org.apache.velocity.app;VelocityEngine;true;mergeTemplate;;;Argument[2];ssti;manual", "org.apache.velocity.runtime.resource.util;StringResourceRepository;true;putStringResource;;;Argument[1];ssti;manual", - "org.apache.velocity.runtime;RuntimeServices;true;evaluate;;;Argument[0];ssti;manual", "org.apache.velocity.runtime;RuntimeServices;true;evaluate;;;Argument[3];ssti;manual", "org.apache.velocity.runtime;RuntimeServices;true;parse;;;Argument[0];ssti;manual", - "org.apache.velocity.runtime;RuntimeSingleton;true;parse;;;Argument[0];ssti;manual", - "org.apache.velocity;Template;true;merge;;;Argument[0];ssti;manual" + "org.apache.velocity.runtime;RuntimeSingleton;true;parse;;;Argument[0];ssti;manual" ] } } diff --git a/java/ql/test/query-tests/security/CWE-094/VelocitySSTI.java b/java/ql/test/query-tests/security/CWE-094/VelocitySSTI.java index b9349e03ddd..09c7a07058f 100644 --- a/java/ql/test/query-tests/security/CWE-094/VelocitySSTI.java +++ b/java/ql/test/query-tests/security/CWE-094/VelocitySSTI.java @@ -61,8 +61,8 @@ public class VelocitySSTI { runtimeServices.parse(reader, new Template()); // $hasTemplateInjection } - @GetMapping(value = "bad4") - public void bad4(HttpServletRequest request) { + @GetMapping(value = "good1") + public void good1(HttpServletRequest request) { String name = "ttemplate"; String code = request.getParameter("code"); @@ -72,7 +72,7 @@ public class VelocitySSTI { StringWriter w = new StringWriter(); StringReader reader = new StringReader("test"); - Velocity.evaluate(context, w, "mystring", reader); // $hasTemplateInjection + Velocity.evaluate(context, w, "mystring", reader); // Safe } @GetMapping(value = "bad5") @@ -85,41 +85,32 @@ public class VelocitySSTI { StringWriter w = new StringWriter(); VelocityEngine engine = null; - engine.mergeTemplate("testtemplate.vm", "UTF-8", context, w); // $hasTemplateInjection + engine.mergeTemplate("testtemplate.vm", "UTF-8", context, w); // Safe AbstractContext ctx = null; ctx.put("key", code); - engine.evaluate(ctx, null, null, null); // $hasTemplateInjection + engine.evaluate(ctx, null, null, (String) null); // Safe + engine.evaluate(ctx, null, null, (Reader) null); // Safe engine.evaluate(null, null, null, code); // $hasTemplateInjection + engine.evaluate(null, null, null, new StringReader(code)); // $hasTemplateInjection + } + + @GetMapping(value = "good2") + public void good2(HttpServletRequest request) { + String name = "ttemplate"; + String code = request.getParameter("code"); + + VelocityContext context = new VelocityContext(); + context.put("code", code); + + StringWriter w = new StringWriter(); + Template t = new Template(); + t.merge(context, w); // Safe + t.merge(context, w, new LinkedList()); // Safe + } @GetMapping(value = "bad6") public void bad6(HttpServletRequest request) { - String name = "ttemplate"; - String code = request.getParameter("code"); - - VelocityContext context = new VelocityContext(); - context.put("code", code); - - StringWriter w = new StringWriter(); - Template t = new Template(); - t.merge(context, w); // $hasTemplateInjection - } - - @GetMapping(value = "bad7") - public void bad7(HttpServletRequest request) { - String name = "ttemplate"; - String code = request.getParameter("code"); - - VelocityContext context = new VelocityContext(); - context.put("code", code); - - StringWriter w = new StringWriter(); - Template t = new Template(); - t.merge(context, w, new LinkedList()); // $hasTemplateInjection - } - - @GetMapping(value = "bad8") - public void bad8(HttpServletRequest request) { String code = request.getParameter("code"); StringResourceRepository repo = new StringResourceRepositoryImpl();