Merge branch 'main' into rc/3.7

This commit is contained in:
Andrew Eisenberg
2022-09-20 08:33:58 -07:00
2309 changed files with 133758 additions and 43219 deletions

View File

@@ -20,7 +20,7 @@ private predicate isDeprecatedCallable(Callable c) {
from Call ca, Callable c
where
ca.getCallee() = c and
ca.getCallee().getSourceDeclaration() = c and
isDeprecatedCallable(c) and
// Exclude deprecated calls from within deprecated code.
not isDeprecatedCallable(ca.getCaller())

View File

@@ -23,7 +23,7 @@ class SuppressionAnnotation extends SuppressWarningsAnnotation {
string text;
SuppressionAnnotation() {
text = this.getASuppressedWarningLiteral().getValue() and
text = this.getASuppressedWarning() and
exists(getAnnotationText(text))
}

View File

@@ -19,6 +19,6 @@ where
eff = t.getMetrics().getEfferentSourceCoupling() and
aff > 15 and
eff > 15
select t as Class,
select t as class_,
"Hub class: this class depends on " + eff.toString() + " classes and is used by " + aff.toString()
+ " classes."

View File

@@ -14,4 +14,4 @@ import semmle.code.java.deadcode.DeadCode
from RootdefCallable c
where not c.whitelisted()
select c.unusedParameter() as p, "The parameter " + p + " is unused."
select c.unusedParameter() as p, "The parameter '" + p + "' is never used."

View File

@@ -7,7 +7,6 @@
* Such operations could interfere with the EJB container's operation.
* @kind problem
* @problem.severity error
* @security-severity 5.8
* @precision low
* @id java/ejb/container-interference
* @tags reliability

View File

@@ -5,7 +5,6 @@
* for enterprise components.
* @kind problem
* @problem.severity error
* @security-severity 5.8
* @precision low
* @id java/ejb/file-io
* @tags reliability

View File

@@ -4,7 +4,6 @@
* Such use could compromise security and system stability.
* @kind problem
* @problem.severity error
* @security-severity 5.8
* @precision low
* @id java/ejb/native-code
* @tags reliability

View File

@@ -4,7 +4,6 @@
* as this could compromise security.
* @kind problem
* @problem.severity error
* @security-severity 5.8
* @precision low
* @id java/ejb/reflection
* @tags external/cwe/cwe-573

View File

@@ -5,7 +5,6 @@
* This functionality is reserved for the EJB container for security reasons.
* @kind problem
* @problem.severity error
* @security-severity 5.8
* @precision low
* @id java/ejb/security-configuration-access
* @tags external/cwe/cwe-573

View File

@@ -4,7 +4,6 @@
* the Java serialization protocol, since their use could compromise security.
* @kind problem
* @problem.severity error
* @security-severity 5.8
* @precision low
* @id java/ejb/substitution-in-serialization
* @tags external/cwe/cwe-573

View File

@@ -5,7 +5,6 @@
* compromise security or interfere with the EJB container's operation.
* @kind problem
* @problem.severity error
* @security-severity 5.8
* @precision low
* @id java/ejb/socket-or-stream-handler-factory
* @tags reliability

View File

@@ -36,11 +36,11 @@ predicate containsSpecialCollection(Expr e, SpecialCollectionCreation origin) {
or
exists(Call c, int i |
containsSpecialCollection(c.getArgument(i), origin) and
e = c.getCallee().getParameter(i).getAnAccess()
e = c.getCallee().getSourceDeclaration().getParameter(i).getAnAccess()
)
or
exists(Call c, ReturnStmt r | e = c |
r.getEnclosingCallable() = c.getCallee() and
r.getEnclosingCallable() = c.getCallee().getSourceDeclaration() and
containsSpecialCollection(r.getResult(), origin)
)
}
@@ -58,11 +58,11 @@ predicate iterOfSpecialCollection(Expr e, SpecialCollectionCreation origin) {
or
exists(Call c, int i |
iterOfSpecialCollection(c.getArgument(i), origin) and
e = c.getCallee().getParameter(i).getAnAccess()
e = c.getCallee().getSourceDeclaration().getParameter(i).getAnAccess()
)
or
exists(Call c, ReturnStmt r | e = c |
r.getEnclosingCallable() = c.getCallee() and
r.getEnclosingCallable() = c.getCallee().getSourceDeclaration() and
iterOfSpecialCollection(r.getResult(), origin)
)
}

View File

@@ -16,4 +16,4 @@ where
e.isStrict() and
e.getGreaterOperand() instanceof BitwiseExpr and
e.getLesserOperand().(IntegerLiteral).getIntValue() = 0
select e, "Sign check of a bitwise operation."
select e, "Potentially unsafe sign check of a bitwise operation."

View File

@@ -77,4 +77,4 @@ where
// Exclude `equals` methods that implement reference-equality.
not m instanceof ReferenceEquals and
not m instanceof UnimplementedEquals
select m, "equals() method does not seem to check argument type."
select m, "equals() method does not check argument type."

View File

@@ -19,8 +19,5 @@ where
m.getNumberOfParameters() = 1 and
c.getArgument(0).getType() = p and
p.getATypeArgument() = t and
not exists(RetentionAnnotation a |
t.getAnAnnotation() = a and
a.getAValue().(VarAccess).getVariable().hasName("RUNTIME")
)
t.getRetentionPolicy() != "RUNTIME"
select c, "Call to isAnnotationPresent where no annotation has the RUNTIME retention policy."

View File

@@ -78,7 +78,7 @@ RefType enclosingInstanceAccess(Expr expr) {
result = ma.getMethod().getDeclaringType() and
not exists(ma.getQualifier()) and
not ma.getMethod().isStatic() and
not exists(Method m | m.getSourceDeclaration() = ma.getMethod() | enclosing.inherits(m))
not enclosing.inherits(ma.getMethod())
)
)
}

View File

@@ -48,6 +48,10 @@ class TaintedPathConfig extends TaintTracking::Configuration {
or
node = DataFlow::BarrierGuard<containsDotDotSanitizer/3>::getABarrierNode()
}
override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
any(TaintedPathAdditionalTaintStep s).step(n1, n2)
}
}
/**

View File

@@ -5,6 +5,49 @@
import java
import semmle.code.java.controlflow.Guards
import semmle.code.java.security.PathCreation
import semmle.code.java.frameworks.Networking
import semmle.code.java.dataflow.DataFlow
/**
* A unit class for adding additional taint steps.
*
* Extend this class to add additional taint steps that should apply to tainted path flow configurations.
*/
class TaintedPathAdditionalTaintStep extends Unit {
abstract predicate step(DataFlow::Node n1, DataFlow::Node n2);
}
private class DefaultTaintedPathAdditionalTaintStep extends TaintedPathAdditionalTaintStep {
override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
exists(Argument a |
a = n1.asExpr() and
a.getCall() = n2.asExpr() and
a = any(TaintPreservingUriCtorParam tpp).getAnArgument()
)
}
}
private class TaintPreservingUriCtorParam extends Parameter {
TaintPreservingUriCtorParam() {
exists(Constructor ctor, int idx, int nParams |
ctor.getDeclaringType() instanceof TypeUri and
this = ctor.getParameter(idx) and
nParams = ctor.getNumberOfParameters()
|
// URI(String scheme, String ssp, String fragment)
idx = 1 and nParams = 3
or
// URI(String scheme, String host, String path, String fragment)
idx = [1, 2] and nParams = 4
or
// URI(String scheme, String authority, String path, String query, String fragment)
idx = 2 and nParams = 5
or
// URI(String scheme, String userInfo, String host, int port, String path, String query, String fragment)
idx = 4 and nParams = 7
)
}
}
private predicate inWeakCheck(Expr e) {
// None of these are sufficient to guarantee that a string is safe.

View File

@@ -27,6 +27,10 @@ class TaintedPathLocalConfig extends TaintTracking::Configuration {
override predicate isSink(DataFlow::Node sink) {
sink.asExpr() = any(PathCreation p).getAnInput()
}
override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
any(TaintedPathAdditionalTaintStep s).step(n1, n2)
}
}
from

View File

@@ -1,8 +1,9 @@
public class XSS extends HttpServlet {
protected void doGet(HttpServletRequest request, HttpServletResponse response)
throws ServletException, IOException {
// BAD: a request parameter is written directly to an error response page
response.sendError(HttpServletResponse.SC_NOT_FOUND,
// BAD: a request parameter is written directly to the Servlet response stream
response.getWriter().print(
"The page \"" + request.getParameter("page") + "\" was not found.");
}
}

View File

@@ -18,7 +18,7 @@ reference.</p>
</recommendation>
<example>
<p>The following example shows the page parameter being written directly to the server error page,
<p>The following example shows the <code>page</code> parameter being written directly to the page,
leaving the website vulnerable to cross-site scripting.</p>
<sample src="XSS.java" />

View File

@@ -19,4 +19,5 @@ import DataFlow::PathGraph
from QueryInjectionSink query, DataFlow::PathNode source, DataFlow::PathNode sink
where queryTaintedBy(query, source, sink)
select query, source, sink, "Query might include code from $@.", source.getNode(), "this user input"
select query, source, sink, "This SQL query depends on $@.", source.getNode(),
"a user-provided value"

View File

@@ -0,0 +1,32 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<overview>
<p>
Template injection occurs when user input is embedded in a template's code in an unsafe manner.
An attacker can use native template syntax to inject a malicious payload into a template, which is then executed server-side.
This permits the attacker to run arbitrary code in the server's context.
</p>
</overview>
<recommendation>
<p>
To fix this, ensure that untrusted input is not used as part of a template's code. If the application requirements do not allow this,
use a sandboxed environment where access to unsafe attributes and methods is prohibited.
</p>
</recommendation>
<example>
<p>
In the example given below, an untrusted HTTP parameter <code>code</code> is used as a Velocity template string.
This can lead to remote code execution.
</p>
<sample src="SSTIBad.java" />
<p>
In the next example, the problem is avoided by using a fixed template string <code>s</code>.
Since the template's code is not attacker-controlled in this case, this solution prevents the execution of untrusted code.
</p>
<sample src="SSTIGood.java" />
</example>
<references>
<li>Portswigger: <a href="https://portswigger.net/web-security/server-side-template-injection">Server Side Template Injection</a>.</li>
</references>
</qhelp>

View File

@@ -1,16 +1,18 @@
/**
* @name Server Side Template Injection
* @description Untrusted input used as a template parameter can lead to remote code execution.
* @name Server-side template injection
* @description Untrusted input interpreted as a template can lead to remote code execution.
* @kind path-problem
* @problem.severity error
* @security-severity 9.3
* @precision high
* @id java/server-side-template-injection
* @tags security
* external/cwe/cwe-1336
* external/cwe/cwe-094
*/
import java
import TemplateInjection
import semmle.code.java.security.TemplateInjectionQuery
import DataFlow::PathGraph
from TemplateInjectionFlowConfig config, DataFlow::PathNode source, DataFlow::PathNode sink

View File

@@ -27,8 +27,21 @@ class ResponseSplittingConfig extends TaintTracking::Configuration {
override predicate isSink(DataFlow::Node sink) { sink instanceof HeaderSplittingSink }
override predicate isSanitizer(DataFlow::Node node) {
node.getType() instanceof PrimitiveType or
node.getType() instanceof PrimitiveType
or
node.getType() instanceof BoxedType
or
exists(MethodAccess ma, string methodName, CompileTimeConstantExpr target |
node.asExpr() = ma and
ma.getMethod().hasQualifiedName("java.lang", "String", methodName) and
target = ma.getArgument(0) and
(
methodName = "replace" and target.getIntValue() = [10, 13] // 10 == "\n", 13 == "\r"
or
methodName = "replaceAll" and
target.getStringValue().regexpMatch(".*([\n\r]|\\[\\^[^\\]\r\n]*\\]).*")
)
)
}
}

View File

@@ -0,0 +1,45 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>In the Android manifest file, you can use the <code>android:allowBackup</code> attribute of the <code>application</code> element to define whether the
application will have automatic backups or not.</p>
<p>If your application uses any sensitive data, you should disable automatic backups to prevent attackers from extracting it.</p>
</overview>
<recommendation>
<p>For Android applications which process sensitive data, set <code>android:allowBackup</code> to <code>false</code> in the manifest
file.</p>
<p>Note: Since Android 6.0 (Marshmallow), automatic backups for applications are switched on by default.
</p>
</recommendation>
<example>
<p>In the following two (bad) examples, the <code>android:allowBackup</code> setting is enabled:</p>
<sample src="AllowBackupTrue.xml" />
<sample src="AllowBackupEmpty.xml"/>
<p>In the following (good) example, <code>android:allowBackup</code> is set to <code>false</code>:</p>
<sample src="AllowBackupFalse.xml"/>
</example>
<references>
<li>
Android Documentation:
<a href="https://developer.android.com/guide/topics/data/autobackup#EnablingAutoBackup">Back up user data with Auto Backup</a>
</li>
<li>
OWASP Mobile Security Testing Guide:
<a href="https://github.com/OWASP/owasp-mstg/blob/b7a93a2e5e0557cc9a12e55fc3f6675f6986bb86/Document/0x05d-Testing-Data-Storage.md#backups">
Android Backups
</a>
</li>
</references>
</qhelp>

View File

@@ -0,0 +1,18 @@
/**
* @name Application backup allowed
* @description Allowing application backups may allow an attacker to extract sensitive data.
* @kind problem
* @problem.severity recommendation
* @security-severity 7.5
* @id java/android/backup-enabled
* @tags security
* external/cwe/cwe-312
* @precision very-high
*/
import java
import semmle.code.xml.AndroidManifest
from AndroidApplicationXmlElement androidAppElem
where androidAppElem.allowsBackup()
select androidAppElem, "Backups are allowed in this Android application."

View File

@@ -0,0 +1,7 @@
<manifest ... >
<!-- BAD: no 'android:allowBackup' set, defaults to 'true' -->
<application>
<activity ... >
</activity>
</application>
</manifest>

View File

@@ -0,0 +1,8 @@
<manifest ... >
<!-- GOOD: 'android:allowBackup' set to 'false' -->
<application
android:allowBackup="false">
<activity ... >
</activity>
</application>
</manifest>

View File

@@ -0,0 +1,8 @@
<manifest ... >
<!-- BAD: 'android:allowBackup' set to 'true' -->
<application
android:allowBackup="true">
<activity ... >
</activity>
</application>
</manifest>

View File

@@ -38,7 +38,7 @@ predicate objectToString(MethodAccess ma) {
exists(ToStringMethod m |
m = ma.getMethod() and
m.getDeclaringType() instanceof TypeObject and
variableTrack(ma.getQualifier()).getType().getErasure() instanceof TypeObject
exprNode(ma.getQualifier()).getTypeBound().getErasure() instanceof TypeObject
)
}

View File

@@ -51,5 +51,6 @@ class XxeConfig extends TaintTracking::Configuration {
from DataFlow::PathNode source, DataFlow::PathNode sink, XxeConfig conf
where conf.hasFlowPath(source, sink)
select sink.getNode(), source, sink, "Unsafe parsing of XML file from $@.", source.getNode(),
"user input"
select sink.getNode(), source, sink,
"A $@ is parsed as XML without guarding against external entity expansion.", source.getNode(),
"user-provided value"

View File

@@ -8,6 +8,7 @@
import java
import semmle.code.java.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
import semmle.code.java.dataflow.internal.NegativeSummary
import ExternalApi
private predicate getRelevantUsages(ExternalApi api, int usages) {

View File

@@ -17,4 +17,4 @@ from JavadocText c
where
c.getText().matches("%TODO%") or
c.getText().matches("%FIXME%")
select c, "TODO/FIXME comment."
select c, "TODO comments should be addressed."

View File

@@ -21,4 +21,4 @@ where
read(v) and
not def.(AssignExpr).getSource() instanceof NullLiteral and
(def instanceof Assignment or def.(UnaryAssignExpr).getParent() instanceof ExprStmt)
select def, "This assignment to " + v.getName() + " is useless: the value is never read."
select def, "This definition of " + v.getName() + " is never used."

View File

@@ -86,5 +86,6 @@ where
) and
// Exclude special VM classes.
not isVMObserver(f.getDeclaringType())
select f, "Field " + f.getName() + " never assigned non-null value, yet it is read at $@.", fr,
fr.getFile().getStem() + ".java:" + fr.getLocation().getStartLine()
select f,
"The field '" + f.getName() + "' is never explicitly assigned a value, yet it is read $@.", fr,
"here"

View File

@@ -15,10 +15,10 @@ import semmle.code.java.NumberFormatException
from Expr e
where
throwsNFE(e) and
throwsNfe(e) and
not exists(TryStmt t |
t.getBlock() = e.getEnclosingStmt().getEnclosingStmt*() and
catchesNFE(t)
catchesNfe(t)
) and
not exists(Callable c |
e.getEnclosingCallable() = c and

View File

@@ -72,7 +72,7 @@ predicate mayWriteToArray(Expr modified) {
// return __array__; ... method()[1] = 0
exists(ReturnStmt rs | modified = rs.getResult() and relevantType(modified.getType()) |
exists(Callable enclosing, MethodAccess ma |
enclosing = rs.getEnclosingCallable() and ma.getMethod() = enclosing
enclosing = rs.getEnclosingCallable() and ma.getMethod().getSourceDeclaration() = enclosing
|
mayWriteToArray(ma)
)
@@ -100,7 +100,7 @@ VarAccess varPassedInto(Callable c, int i) {
predicate exposesByReturn(Callable c, Field f, Expr why, string whyText) {
returnsArray(c, f) and
exists(MethodAccess ma |
ma.getMethod() = c and ma.getCompilationUnit() != c.getCompilationUnit()
ma.getMethod().getSourceDeclaration() = c and ma.getCompilationUnit() != c.getCompilationUnit()
|
mayWriteToArray(ma) and
why = ma and

View File

@@ -41,4 +41,4 @@ where
f.getType() instanceof Array and
f.fromSource() and
forall(AssignExpr a | a.getDest() = f.getAnAccess() | nonEmptyArrayLiteralOrNull(a.getSource()))
select f, "The array constant " + f.getName() + " is vulnerable to mutation."
select f, "The array constant '" + f.getName() + "' is vulnerable to mutation."

View File

@@ -27,7 +27,7 @@ predicate callToInheritedMethod(RefType lexicalScope, MethodAccess ma, string si
not ma.getMethod().isStatic() and
not ma.hasQualifier() and
ma.getEnclosingCallable().getDeclaringType() = lexicalScope and
nestedSupertypePlus(lexicalScope).getAMethod() = ma.getMethod() and
nestedSupertypePlus(lexicalScope).getAMethod() = ma.getMethod().getSourceDeclaration() and
signature = ma.getMethod().getSignature()
}

View File

@@ -71,4 +71,4 @@ where
not neededImport(i) and
not i instanceof ImportStaticOnDemand and
not i instanceof ImportStaticTypeMember
select i, "The statement '" + i + "' is unnecessary."
select i, "Import of '" + i + "' is not used."

View File

@@ -0,0 +1,4 @@
---
category: newQuery
---
* Added a new query, `java/android/backup-enabled`, to detect if Android applications allow backups.

View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The alert message of many queries have been changed to make the message consistent with other languages.

View File

@@ -0,0 +1,4 @@
---
category: queryMetadata
---
* Removed the `@security-severity` tag from several queries not in the `Security/` folder that also had missing `security` tags.

View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Added new sinks related to Android's `AlarmManager` to the query `java/android/implicit-pendingintents`.

View File

@@ -0,0 +1,4 @@
---
category: newQuery
---
* The query "Server-side template injection" (`java/server-side-template-injection`) has been promoted from experimental to the main query pack. This query was originally [submitted as an experimental query by @porcupineyhairs](https://github.com/github/codeql/pull/5935).

View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Added taint model for arguments of `java.net.URI` constructors to the queries `java/path-injection` and `java/path-injection-local`.

View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The Java extractor now populates the `Method` relating to a `MethodAccess` consistently for calls using an explicit and implicit `this` qualifier. Previously if the method `foo` was inherited from a specialised generic type `ParentType<String>`, then an explicit call `this.foo()` would yield a `MethodAccess` whose `getMethod()` accessor returned the bound method `ParentType<String>.foo`, whereas an implicitly-qualified `foo()` `MethodAccess`'s `getMethod()` would return the unbound method `ParentType.foo`. Now both scenarios produce a bound method. This means that all data-flow queries may return more results where a relevant path transits a call to such an implicitly-qualified call to a member method with a bound generic type, while queries that inspect the result of `MethodAccess.getMethod()` may need to tolerate bound generic methods in more circumstances. The queries `java/iterator-remove-failure`, `java/non-static-nested-class`, `java/internal-representation-exposure`, `java/subtle-inherited-call` and `java/deprecated-call` have been amended to properly handle calls to bound generic methods, and in some instances may now produce more results in the explicit-`this` case as well.

View File

@@ -1,31 +0,0 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<overview>
<p>
Template Injection occurs when user input is embedded in a template in an unsafe manner.
An attacker can use native template syntax to inject a malicious payload into a template, which is then executed server-side. This permits the attacker to run arbitrary code in the server's context.</p>
</overview>
<recommendation>
<p>
To fix this, ensure that an untrusted value is not used as a template. If the application requirements do not allow this, use a sandboxed environment where access to unsafe attributes and methods is prohibited.
</p>
</recommendation>
<example>
<p>
In the example given below, an untrusted HTTP parameter
<code>code</code>
is used as a Velocity template string. This can lead to remote code execution.
</p>
<sample src="SSTIBad.java" />
<p>
In the next example the problem is avoided by using a fixed template string
<code>s</code>
. Since, the template is not attacker controlled in this case, we prevent untrusted code execution.
</p>
<sample src="SSTIGood.java" />
</example>
<references>
<li>Portswigger : [Server Side Template Injection](https://portswigger.net/web-security/server-side-template-injection)</li>
</references>
</qhelp>

View File

@@ -1,209 +0,0 @@
/** Definitions related to the Server Side Template Injection (SSTI) query. */
import java
import semmle.code.java.dataflow.TaintTracking
import semmle.code.java.dataflow.FlowSources
import experimental.semmle.code.java.frameworks.FreeMarker
import experimental.semmle.code.java.frameworks.Velocity
import experimental.semmle.code.java.frameworks.JinJava
import experimental.semmle.code.java.frameworks.Pebble
import experimental.semmle.code.java.frameworks.Thymeleaf
/** A taint tracking configuration to reason about Server Side Template Injection (SSTI) vulnerabilities */
class TemplateInjectionFlowConfig extends TaintTracking::Configuration {
TemplateInjectionFlowConfig() { this = "TemplateInjectionFlowConfig" }
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
override predicate isSanitizer(DataFlow::Node node) {
node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
}
override predicate isAdditionalTaintStep(DataFlow::Node prev, DataFlow::Node succ) {
exists(AdditionalFlowStep a | a.isAdditionalTaintStep(prev, succ))
}
}
/**
* A data flow sink for Server Side Template Injection (SSTI) vulnerabilities
*/
abstract private class Sink extends DataFlow::ExprNode { }
/**
* A data flow step for Server Side Template Injection (SSTI) vulnerabilities
*/
private class AdditionalFlowStep extends Unit {
abstract predicate isAdditionalTaintStep(DataFlow::Node prev, DataFlow::Node succ);
}
/**
* An argument to FreeMarker template engine's `process` method call.
*/
private class FreeMarkerProcessSink extends Sink {
FreeMarkerProcessSink() {
exists(MethodAccess m |
m.getCallee() instanceof MethodFreeMarkerTemplateProcess and
m.getArgument(0) = this.getExpr()
)
}
}
/**
* An reader passed an argument to FreeMarker template engine's `Template`
* construtor call.
*/
private class FreeMarkerConstructorSink extends Sink {
FreeMarkerConstructorSink() {
// Template(java.lang.String name, java.io.Reader reader)
// Template(java.lang.String name, java.io.Reader reader, Configuration cfg)
// Template(java.lang.String name, java.io.Reader reader, Configuration cfg, java.lang.String encoding)
// Template(java.lang.String name, java.lang.String sourceName, java.io.Reader reader, Configuration cfg)
// Template(java.lang.String name, java.lang.String sourceName, java.io.Reader reader, Configuration cfg, ParserConfiguration customParserConfiguration, java.lang.String encoding)
// Template(java.lang.String name, java.lang.String sourceName, java.io.Reader reader, Configuration cfg, java.lang.String encoding)
exists(ConstructorCall cc, Expr e |
cc.getConstructor().getDeclaringType() instanceof TypeFreeMarkerTemplate and
e = cc.getAnArgument() and
(
e.getType().(RefType).hasQualifiedName("java.io", "Reader") and
this.asExpr() = e
)
)
or
exists(ConstructorCall cc |
cc.getConstructor().getDeclaringType() instanceof TypeFreeMarkerTemplate and
// Template(java.lang.String name, java.lang.String sourceCode, Configuration cfg)
cc.getNumArgument() = 3 and
cc.getArgument(1).getType() instanceof TypeString and
this.asExpr() = cc.getArgument(1)
)
}
}
/**
* An argument to FreeMarker template engine's `putTemplate` method call.
*/
private class FreeMarkerStringTemplateLoaderPutTemplateSink extends Sink {
FreeMarkerStringTemplateLoaderPutTemplateSink() {
exists(MethodAccess ma |
this.asExpr() = ma.getArgument(1) and
ma.getMethod() instanceof MethodFreeMarkerStringTemplateLoaderPutTemplate
)
}
}
/**
* An argument to Pebble template engine's `getLiteralTemplate` or `getTemplate` method call.
*/
private class PebbleGetTemplateSinkTemplateSink extends Sink {
PebbleGetTemplateSinkTemplateSink() {
exists(MethodAccess ma |
this.asExpr() = ma.getArgument(0) and
ma.getMethod() instanceof MethodPebbleGetTemplate
)
}
}
/**
* An argument to JinJava template engine's `render` or `renderForResult` method call.
*/
private class JinjavaRenderSink extends Sink {
JinjavaRenderSink() {
exists(MethodAccess ma |
this.asExpr() = ma.getArgument(0) and
(
ma.getMethod() instanceof MethodJinjavaRenderForResult
or
ma.getMethod() instanceof MethodJinjavaRender
)
)
}
}
/**
* An argument to ThymeLeaf template engine's `process` method call.
*/
private class ThymeLeafRenderSink extends Sink {
ThymeLeafRenderSink() {
exists(MethodAccess ma |
this.asExpr() = ma.getArgument(0) and
ma.getMethod() instanceof MethodThymeleafProcess
)
}
}
/**
* Tainted data flowing into a Velocity Context through `put` method taints the context.
*/
private class VelocityContextFlow extends AdditionalFlowStep {
override predicate isAdditionalTaintStep(DataFlow::Node prev, DataFlow::Node succ) {
exists(MethodAccess m | m.getMethod() instanceof MethodVelocityContextPut |
m.getArgument(1) = prev.asExpr() and
succ.asExpr() = m.getQualifier()
)
}
}
/**
* An argument to Velocity template engine's `mergeTemplate` method call.
*/
private class VelocityMergeTempSink extends Sink {
VelocityMergeTempSink() {
exists(MethodAccess m |
// static boolean mergeTemplate(String templateName, String encoding, Context context, Writer writer)
m.getCallee() instanceof MethodVelocityMergeTemplate and
m.getArgument(2) = this.getExpr()
)
}
}
/**
* An argument to Velocity template engine's `mergeTemplate` method call.
*/
private class VelocityMergeSink extends Sink {
VelocityMergeSink() {
exists(MethodAccess m |
m.getCallee() instanceof MethodVelocityMerge and
// public void merge(Context context, Writer writer)
// public void merge(Context context, Writer writer, List<String> macroLibraries)
m.getArgument(0) = this.getExpr()
)
}
}
/**
* An argument to Velocity template engine's `evaluate` method call.
*/
private class VelocityEvaluateSink extends Sink {
VelocityEvaluateSink() {
exists(MethodAccess m |
m.getCallee() instanceof MethodVelocityEvaluate and
m.getArgument([0, 3]) = this.getExpr()
)
}
}
/**
* An argument to Velocity template engine's `parse` method call.
*/
private class VelocityParseSink extends Sink {
VelocityParseSink() {
exists(MethodAccess ma |
this.asExpr() = ma.getArgument(0) and
ma.getMethod() instanceof MethodVelocityParse
)
}
}
/**
* An argument to Velocity template engine's `putStringResource` method call.
*/
private class VelocityPutStringResSink extends Sink {
VelocityPutStringResSink() {
exists(MethodAccess ma |
this.asExpr() = ma.getArgument(1) and
ma.getMethod() instanceof MethodVelocityPutStringResource
)
}
}

View File

@@ -117,12 +117,12 @@ predicate hasShortAsymmetricKeyPair(MethodAccess ma, string msg, string type) {
}
/** Holds if a DSA `KeyPairGenerator` initialized by `ma` uses an insufficient key size. `msg` provides a human-readable description of the problem. */
predicate hasShortDSAKeyPair(MethodAccess ma, string msg) {
predicate hasShortDsaKeyPair(MethodAccess ma, string msg) {
hasShortAsymmetricKeyPair(ma, msg, "DSA") or hasShortAsymmetricKeyPair(ma, msg, "DH")
}
/** Holds if a RSA `KeyPairGenerator` initialized by `ma` uses an insufficient key size. `msg` provides a human-readable description of the problem. */
predicate hasShortRSAKeyPair(MethodAccess ma, string msg) {
predicate hasShortRsaKeyPair(MethodAccess ma, string msg) {
hasShortAsymmetricKeyPair(ma, msg, "RSA")
}
@@ -147,7 +147,7 @@ predicate hasShortECKeyPair(MethodAccess ma, string msg) {
from Expr e, string msg
where
hasShortAESKey(e, msg) or
hasShortDSAKeyPair(e, msg) or
hasShortRSAKeyPair(e, msg) or
hasShortDsaKeyPair(e, msg) or
hasShortRsaKeyPair(e, msg) or
hasShortECKeyPair(e, msg)
select e, msg

View File

@@ -47,8 +47,8 @@ class SpringControllerRequestMappingGetMethod extends SpringControllerGetMethod
.getType()
.hasQualifiedName("org.springframework.web.bind.annotation", "RequestMapping") and
(
this.getAnAnnotation().getValue("method").(VarAccess).getVariable().getName() = "GET" or
this.getAnAnnotation().getValue("method").(ArrayInit).getSize() = 0 //Java code example: @RequestMapping(value = "test")
this.getAnAnnotation().getAnEnumConstantArrayValue("method").getName() = "GET" or
not exists(this.getAnAnnotation().getAnArrayValue("method")) //Java code example: @RequestMapping(value = "test")
) and
not this.getAParamType().getName() = "MultipartFile"
}

View File

@@ -48,4 +48,4 @@ where
)
or
hasEmbeddedPassword(nameAttr.getValue().trim()) // Attribute value matches password pattern
select nameAttr, "Plaintext password in configuration file."
select nameAttr, "Avoid plaintext passwords in configuration files."

View File

@@ -11,7 +11,7 @@
*/
import java
import SpringUrlRedirect
import experimental.semmle.code.java.security.SpringUrlRedirect
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.controlflow.Guards
import DataFlow::PathGraph

View File

@@ -0,0 +1,32 @@
String PROTECTED_PATTERN = "/protected/.*";
String CONSTRAINT_PATTERN = "/protected/xyz\\.xml";
// BAD: A string with line return e.g. `/protected/%0dxyz` can bypass the path check
Pattern p = Pattern.compile(PROTECTED_PATTERN);
Matcher m = p.matcher(path);
// GOOD: A string with line return e.g. `/protected/%0dxyz` cannot bypass the path check
Pattern p = Pattern.compile(PROTECTED_PATTERN, Pattern.DOTALL);
Matcher m = p.matcher(path);
// GOOD: Only a specific path can pass the validation
Pattern p = Pattern.compile(CONSTRAINT_PATTERN);
Matcher m = p.matcher(path);
if (m.matches()) {
// Protected page - check access token and redirect to login page
} else {
// Not protected page - render content
}
// BAD: A string with line return e.g. `/protected/%0axyz` can bypass the path check
boolean matches = path.matches(PROTECTED_PATTERN);
// BAD: A string with line return e.g. `/protected/%0axyz` can bypass the path check
boolean matches = Pattern.matches(PROTECTED_PATTERN, path);
if (matches) {
// Protected page - check access token and redirect to login page
} else {
// Not protected page - render content
}

View File

@@ -0,0 +1,37 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>By default, a "dot" (<code>.</code>) in a regular expression matches all characters except the newline characters <code>\n</code> and
<code>\r</code>. Regular expressions containing a dot can be bypassed with the characters <code>\r</code>(<code>%0a</code>) and
<code>\n</code>(<code>%0d</code>) when the default Java regular expression matching implementations are used. This becomes a security issue
if these regular expressions are used to decide whether to grant access to protected application resources.</p>
</overview>
<recommendation>
<p>To guard against unauthorized access, it is advisable to properly specify regex patterns for validating user input. The Java
Pattern Matcher API <code>Pattern.compile(PATTERN, Pattern.DOTALL)</code> with the <code>DOTALL</code> flag set can be adopted
to address this vulnerability.</p>
</recommendation>
<example>
<p>The following snippets show a vulnerable example and a secure example respectively. The <code>bad</code> methods show a regex pattern allowing
a bypass by using line break characters. In the <code>good</code> methods, it is shown how to solve this problem by either specifying the regex
pattern correctly or using a Java API that properly matches new line characters.
</p>
<sample src="DotRegex.java" />
</example>
<references>
<li>Apache Shiro:
<a href="https://github.com/apache/shiro/commit/6bcb92e06fa588b9c7790dd01bc02135d58d3f5b">Address the RegexRequestMatcher issue in 1.9.1</a>.
</li>
<li>CVE-2022-32532:
<a href="https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-32532">Applications using RegExPatternMatcher with "." in the regular expression are possibly vulnerable to an authorization bypass</a>.
</li>
</references>
</qhelp>

View File

@@ -0,0 +1,22 @@
/**
* @name URL matched by permissive `.` in a regular expression
* @description URLs validated with a permissive `.` in regular expressions may be vulnerable
* to an authorization bypass.
* @kind path-problem
* @problem.severity warning
* @precision high
* @id java/permissive-dot-regex
* @tags security
* external/cwe-625
* external/cwe-863
*/
import java
import semmle.code.java.dataflow.FlowSources
import DataFlow::PathGraph
import PermissiveDotRegexQuery
from DataFlow::PathNode source, DataFlow::PathNode sink, MatchRegexConfiguration conf
where conf.hasFlowPath(source, sink)
select sink.getNode(), source, sink, "Potentially authentication bypass due to $@.",
source.getNode(), "user-provided value"

View File

@@ -0,0 +1,223 @@
/** Provides classes related to security-centered regular expression matching. */
import java
private import semmle.code.java.dataflow.ExternalFlow
private import semmle.code.java.dataflow.FlowSources
import experimental.semmle.code.java.security.SpringUrlRedirect
import semmle.code.java.controlflow.Guards
import semmle.code.java.security.UrlRedirect
import Regex
/** A string that ends with `.*` not prefixed with `\`. */
private class PermissiveDotStr extends StringLiteral {
PermissiveDotStr() {
exists(string s, int i | this.getValue() = s |
s.indexOf(".*") = i and
not s.charAt(i - 1) = "\\" and
s.length() = i + 2
)
}
}
/** Remote flow sources obtained from the URI of a servlet request. */
private class GetServletUriSource extends SourceModelCsv {
override predicate row(string row) {
row =
[
"javax.servlet.http;HttpServletRequest;false;getPathInfo;();;ReturnValue;uri-path;manual",
"javax.servlet.http;HttpServletRequest;false;getPathTranslated;();;ReturnValue;uri-path;manual",
"javax.servlet.http;HttpServletRequest;false;getRequestURI;();;ReturnValue;uri-path;manual",
"javax.servlet.http;HttpServletRequest;false;getRequestURL;();;ReturnValue;uri-path;manual",
"javax.servlet.http;HttpServletRequest;false;getServletPath;();;ReturnValue;uri-path;manual"
]
}
}
/** The qualifier of a request dispatch method call. */
private class UrlDispatchSink extends UrlRedirectSink {
UrlDispatchSink() {
exists(MethodAccess ma |
ma.getMethod() instanceof RequestDispatchMethod and
this.asExpr() = ma.getQualifier()
)
}
}
/** The `doFilter` method of `javax.servlet.FilterChain`. */
private class ServletFilterMethod extends Method {
ServletFilterMethod() {
this.getDeclaringType().getASupertype*().hasQualifiedName("javax.servlet", "FilterChain") and
this.hasName("doFilter")
}
}
/** The qualifier of a servlet filter method call. */
private class UrlFilterSink extends UrlRedirectSink {
UrlFilterSink() {
exists(MethodAccess ma |
ma.getMethod() instanceof ServletFilterMethod and
this.asExpr() = ma.getQualifier()
)
}
}
/** A Spring framework annotation indicating that a URI is user-provided. */
private class SpringUriInputAnnotation extends Annotation {
SpringUriInputAnnotation() {
this.getType()
.hasQualifiedName("org.springframework.web.bind.annotation",
["PathVariable", "RequestParam"])
}
}
/** A user-provided URI parameter of a request mapping method. */
private class SpringUriInputParameterSource extends DataFlow::Node {
SpringUriInputParameterSource() {
this.asParameter() =
any(SpringRequestMappingParameter srmp |
srmp.getAnAnnotation() instanceof SpringUriInputAnnotation
)
}
}
/**
* A data flow sink to construct regular expressions.
*/
private class CompileRegexSink extends DataFlow::ExprNode {
CompileRegexSink() {
exists(MethodAccess ma, Method m | m = ma.getMethod() |
(
ma.getArgument(0) = this.asExpr() and
(
m instanceof StringMatchMethod // input.matches(regexPattern)
or
m instanceof PatternCompileMethod // p = Pattern.compile(regexPattern)
or
m instanceof PatternMatchMethod // p = Pattern.matches(regexPattern, input)
)
)
)
}
}
/**
* A data flow configuration for regular expressions that include permissive dots.
*/
private class PermissiveDotRegexConfig extends DataFlow2::Configuration {
PermissiveDotRegexConfig() { this = "PermissiveDotRegex::PermissiveDotRegexConfig" }
override predicate isSource(DataFlow2::Node src) { src.asExpr() instanceof PermissiveDotStr }
override predicate isSink(DataFlow2::Node sink) { sink instanceof CompileRegexSink }
override predicate isBarrier(DataFlow2::Node node) {
exists(
MethodAccess ma, Field f // Pattern.compile(PATTERN, Pattern.DOTALL)
|
ma.getMethod() instanceof PatternCompileMethod and
ma.getArgument(1) = f.getAnAccess() and
f.hasName("DOTALL") and
f.getDeclaringType() instanceof Pattern and
node.asExpr() = ma.getArgument(0)
)
}
}
/**
* A taint-tracking configuration for untrusted user input used to match regular expressions
* that include permissive dots.
*/
class MatchRegexConfiguration extends TaintTracking::Configuration {
MatchRegexConfiguration() { this = "PermissiveDotRegex::MatchRegexConfiguration" }
override predicate isSource(DataFlow::Node source) {
sourceNode(source, "uri-path") or // Servlet uri source
source instanceof SpringUriInputParameterSource // Spring uri source
}
override predicate isSink(DataFlow::Node sink) {
sink instanceof MatchRegexSink and
exists(
Guard guard, Expr se, Expr ce // used in a condition to control url redirect, which is a typical security enforcement
|
(
sink.asExpr() = ce.(MethodAccess).getQualifier() or
sink.asExpr() = ce.(MethodAccess).getAnArgument() or
sink.asExpr() = ce
) and
(
DataFlow::localExprFlow(ce, guard.(MethodAccess).getQualifier()) or
DataFlow::localExprFlow(ce, guard.(MethodAccess).getAnArgument())
) and
(
DataFlow::exprNode(se) instanceof UrlRedirectSink or
DataFlow::exprNode(se) instanceof SpringUrlRedirectSink
) and
guard.controls(se.getBasicBlock(), true)
) and
exists(MethodAccess ma | any(PermissiveDotRegexConfig conf2).hasFlowToExpr(ma.getArgument(0)) |
// input.matches(regexPattern)
ma.getMethod() instanceof StringMatchMethod and
ma.getQualifier() = sink.asExpr()
or
// p = Pattern.compile(regexPattern); p.matcher(input)
ma.getMethod() instanceof PatternCompileMethod and
exists(MethodAccess pma |
pma.getMethod() instanceof PatternMatcherMethod and
sink.asExpr() = pma.getArgument(0) and
DataFlow::localExprFlow(ma, pma.getQualifier())
)
or
// p = Pattern.matches(regexPattern, input)
ma.getMethod() instanceof PatternMatchMethod and
sink.asExpr() = ma.getArgument(1)
)
}
}
/**
* A data flow sink representing a string being matched against a regular expression.
*/
abstract class MatchRegexSink extends DataFlow::ExprNode { }
/**
* A string being matched against a regular expression.
*/
private class StringMatchRegexSink extends MatchRegexSink {
StringMatchRegexSink() {
exists(MethodAccess ma, Method m | m = ma.getMethod() |
(
m instanceof StringMatchMethod and
ma.getQualifier() = this.asExpr()
)
)
}
}
/**
* A string being matched against a regular expression using a pattern.
*/
private class PatternMatchRegexSink extends MatchRegexSink {
PatternMatchRegexSink() {
exists(MethodAccess ma, Method m | m = ma.getMethod() |
(
m instanceof PatternMatchMethod and
ma.getArgument(1) = this.asExpr()
)
)
}
}
/**
* A string being used to create a pattern matcher.
*/
private class PatternMatcherRegexSink extends MatchRegexSink {
PatternMatcherRegexSink() {
exists(MethodAccess ma, Method m | m = ma.getMethod() |
(
m instanceof PatternMatcherMethod and
ma.getArgument(0) = this.asExpr()
)
)
}
}

View File

@@ -0,0 +1,50 @@
/** Provides methods related to regular expression matching. */
import java
/**
* The class `java.util.regex.Pattern`.
*/
class Pattern extends RefType {
Pattern() { this.hasQualifiedName("java.util.regex", "Pattern") }
}
/**
* The method `compile` of `java.util.regex.Pattern`.
*/
class PatternCompileMethod extends Method {
PatternCompileMethod() {
this.getDeclaringType().getASupertype*() instanceof Pattern and
this.hasName("compile")
}
}
/**
* The method `matches` of `java.util.regex.Pattern`.
*/
class PatternMatchMethod extends Method {
PatternMatchMethod() {
this.getDeclaringType().getASupertype*() instanceof Pattern and
this.hasName("matches")
}
}
/**
* The method `matcher` of `java.util.regex.Pattern`.
*/
class PatternMatcherMethod extends Method {
PatternMatcherMethod() {
this.getDeclaringType().getASupertype*() instanceof Pattern and
this.hasName("matcher")
}
}
/**
* The method `matches` of `java.lang.String`.
*/
class StringMatchMethod extends Method {
StringMatchMethod() {
this.getDeclaringType().getASupertype*() instanceof TypeString and
this.hasName("matches")
}
}

View File

@@ -85,5 +85,5 @@ class RegexInjectionConfiguration extends TaintTracking::Configuration {
from DataFlow::PathNode source, DataFlow::PathNode sink, RegexInjectionConfiguration c
where c.hasFlowPath(source, sink)
select sink.getNode(), source, sink, "$@ is user controlled.", source.getNode(),
"This regular expression pattern"
select sink.getNode(), source, sink, "This regular expression is constructed from a $@.",
source.getNode(), "user-provided value"

View File

@@ -21,8 +21,8 @@ import DataFlow::PathGraph
/**
* Taint configuration tracking flow from untrusted inputs to number conversion calls in exported Android compononents.
*/
class NFELocalDoSConfiguration extends TaintTracking::Configuration {
NFELocalDoSConfiguration() { this = "NFELocalDoSConfiguration" }
class NfeLocalDoSConfiguration extends TaintTracking::Configuration {
NfeLocalDoSConfiguration() { this = "NFELocalDoSConfiguration" }
/** Holds if source is a remote flow source */
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -31,17 +31,17 @@ class NFELocalDoSConfiguration extends TaintTracking::Configuration {
override predicate isSink(DataFlow::Node sink) {
exists(Expr e |
e.getEnclosingCallable().getDeclaringType().(ExportableAndroidComponent).isExported() and
throwsNFE(e) and
throwsNfe(e) and
not exists(TryStmt t |
t.getBlock() = e.getAnEnclosingStmt() and
catchesNFE(t)
catchesNfe(t)
) and
sink.asExpr() = e
)
}
}
from DataFlow::PathNode source, DataFlow::PathNode sink, NFELocalDoSConfiguration conf
from DataFlow::PathNode source, DataFlow::PathNode sink, NfeLocalDoSConfiguration conf
where conf.hasFlowPath(source, sink)
select sink.getNode(), source, sink,
"Uncaught NumberFormatException in an exported Android component due to $@.", source.getNode(),

View File

@@ -1,29 +0,0 @@
/** Definitions related to the FreeMarker Templating library. */
import java
/** The `Template` class of the FreeMarker Template Engine */
class TypeFreeMarkerTemplate extends Class {
TypeFreeMarkerTemplate() { this.hasQualifiedName("freemarker.template", "Template") }
}
/** The `process` method of the FreeMarker Template Engine's `Template` class */
class MethodFreeMarkerTemplateProcess extends Method {
MethodFreeMarkerTemplateProcess() {
this.getDeclaringType() instanceof TypeFreeMarkerTemplate and
this.hasName("process")
}
}
/** The `StringTemplateLoader` class of the FreeMarker Template Engine */
class TypeFreeMarkerStringLoader extends Class {
TypeFreeMarkerStringLoader() { this.hasQualifiedName("freemarker.cache", "StringTemplateLoader") }
}
/** The `process` method of the FreeMarker Template Engine's `StringTemplateLoader` class */
class MethodFreeMarkerStringTemplateLoaderPutTemplate extends Method {
MethodFreeMarkerStringTemplateLoaderPutTemplate() {
this.getDeclaringType() instanceof TypeFreeMarkerStringLoader and
this.hasName("putTemplate")
}
}

View File

@@ -1,24 +0,0 @@
/** Definitions related to the Jinjava Templating library. */
import java
/** The `Jinjava` class of the Jinjava Templating Engine. */
class TypeJinjava extends Class {
TypeJinjava() { this.hasQualifiedName("com.hubspot.jinjava", "Jinjava") }
}
/** The `render` method of the Jinjava Templating Engine. */
class MethodJinjavaRender extends Method {
MethodJinjavaRender() {
this.getDeclaringType() instanceof TypeJinjava and
this.hasName("render")
}
}
/** The `render` method of the Jinjava Templating Engine. */
class MethodJinjavaRenderForResult extends Method {
MethodJinjavaRenderForResult() {
this.getDeclaringType() instanceof TypeJinjava and
this.hasName("renderForResult")
}
}

View File

@@ -1,16 +0,0 @@
/** Definitions related to the Pebble Templating library. */
import java
/** The `PebbleEngine` class of the Pebble Templating Engine. */
class TypePebbleEngine extends Class {
TypePebbleEngine() { this.hasQualifiedName("com.mitchellbosecke.pebble", "PebbleEngine") }
}
/** The `getTemplate` method of the Pebble Templating Engine. */
class MethodPebbleGetTemplate extends Method {
MethodPebbleGetTemplate() {
this.getDeclaringType() instanceof TypePebbleEngine and
this.hasName(["getTemplate", "getLiteralTemplate"])
}
}

View File

@@ -1,25 +0,0 @@
/** Definitions related to the Thymeleaf Templating library. */
import java
/**
* A class implementing the `ITemplateEngine` interface of the Thymeleaf
* Templating Engine such as the `TemplateEngine` class.
*/
class TypeThymeleafTemplateEngine extends Class {
TypeThymeleafTemplateEngine() {
this.hasQualifiedName("org.thymeleaf", "TemplateEngine")
or
exists(Type t | this.getASupertype*().extendsOrImplements(t) |
t.hasName("org.thymeleaf.ITemplateEngine")
)
}
}
/** The `process` or `processThrottled` method of the Thymeleaf Templating Engine. */
class MethodThymeleafProcess extends Method {
MethodThymeleafProcess() {
this.getDeclaringType() instanceof TypeThymeleafTemplateEngine and
this.hasName(["process", "processThrottled"])
}
}

View File

@@ -1,119 +0,0 @@
/** Definitions related to the Apache Velocity Templating library. */
import java
/** The `org.apache.velocity.context.AbstractContext` class of the Velocity Templating Engine. */
class TypeVelocityAbstractContext extends Class {
TypeVelocityAbstractContext() {
this.hasQualifiedName("org.apache.velocity.context", "AbstractContext")
}
}
/** The `org.apache.velocity.runtime.RuntimeServices` class of the Velocity Templating Engine. */
class TypeVelocityRuntimeRuntimeServices extends Class {
TypeVelocityRuntimeRuntimeServices() {
this.hasQualifiedName("org.apache.velocity.runtime", "RuntimeServices")
}
}
/** The `org.apache.velocity.Template` class of the Velocity Templating Engine. */
class TypeVelocityTemplate extends Class {
TypeVelocityTemplate() { this.hasQualifiedName("org.apache.velocity", "Template") }
}
/** The `org.apache.velocity.runtime.RuntimeSingleton` classTemplating Engine. */
class TypeVelocityRuntimeRuntimeSingleton extends Class {
TypeVelocityRuntimeRuntimeSingleton() {
this.hasQualifiedName("org.apache.velocity.runtime", "RuntimeSingleton")
}
}
/** The `org.apache.velocity.VelocityEngine` class of the Velocity Templating Engine. */
class TypeVelocityVelocityEngine extends Class {
TypeVelocityVelocityEngine() { this.hasQualifiedName("org.apache.velocity", "VelocityEngine") }
}
/** The `org.apache.velocity.app.VelocityEngine` class of the Velocity Templating Engine. */
class TypeVelocityAppVelocityEngine extends RefType {
TypeVelocityAppVelocityEngine() {
this.hasQualifiedName("org.apache.velocity.app", "VelocityEngine")
}
}
/** The `org.apache.velocity.app.Velocity` class of the Velocity Templating Engine. */
class TypeVelocityAppVelocity extends RefType {
TypeVelocityAppVelocity() { this.hasQualifiedName("org.apache.velocity.app", "Velocity") }
}
/**
* The `org.apache.velocity.runtime.resource.util.StringResourceRepository` interface
* of the Velocity Templating Engine.
*/
class TypeVelocityStringResourceRepo extends RefType {
TypeVelocityStringResourceRepo() {
this.hasQualifiedName("org.apache.velocity.runtime.resource.util", "StringResourceRepository")
}
}
/** The `internalPut` and `put` methods of the Velocity Templating Engine. */
class MethodVelocityContextPut extends Method {
MethodVelocityContextPut() {
this.getDeclaringType().getASupertype*() instanceof TypeVelocityAbstractContext and
this.hasName(["put", "internalPut"])
}
}
/** The `evaluate` method of the Velocity Templating Engine. */
class MethodVelocityEvaluate extends Method {
MethodVelocityEvaluate() {
// static boolean evaluate(Context context, Writer out, String logTag, String instring)
// static boolean evaluate(Context context, Writer writer, String logTag, Reader reader)
(
this.getDeclaringType() instanceof TypeVelocityAppVelocity or
this.getDeclaringType() instanceof TypeVelocityAppVelocityEngine or
this.getDeclaringType().getASupertype*() instanceof TypeVelocityRuntimeRuntimeServices
) and
this.hasName("evaluate")
}
}
/** The `mergeTemplate` method of the Velocity Templating Engine. */
class MethodVelocityMergeTemplate extends Method {
MethodVelocityMergeTemplate() {
// static boolean mergeTemplate(String templateName, String encoding, Context context, Writer writer)
(
this.getDeclaringType() instanceof TypeVelocityAppVelocity or
this.getDeclaringType() instanceof TypeVelocityAppVelocityEngine
) and
this.hasName("mergeTemplate")
}
}
/** The `merge` method of the Velocity Templating Engine. */
class MethodVelocityMerge extends Method {
MethodVelocityMerge() {
// void merge(Context context, Writer writer)
// void merge(Context context, Writer writer, List<String> macroLibraries)
this.getDeclaringType() instanceof TypeVelocityTemplate and
this.hasName("merge")
}
}
/** The `parse` method of the Velocity Templating Engine. */
class MethodVelocityParse extends Method {
MethodVelocityParse() {
(
this.getDeclaringType().getASupertype*() instanceof TypeVelocityRuntimeRuntimeSingleton or
this.getDeclaringType().getASupertype*() instanceof TypeVelocityRuntimeRuntimeServices
) and
this.hasName("parse")
}
}
/** The `putStringResource` method of the Velocity Templating Engine. */
class MethodVelocityPutStringResource extends Method {
MethodVelocityPutStringResource() {
this.getDeclaringType().getASupertype*() instanceof TypeVelocityStringResourceRepo and
this.hasName("putStringResource")
}
}

View File

@@ -1,9 +1,7 @@
import java
import DataFlow
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.dataflow.DataFlow2
import semmle.code.java.dataflow.TaintTracking
import semmle.code.java.frameworks.spring.SpringController
/** Provides classes and predicates related to Spring URL redirect. */
private import java
private import semmle.code.java.dataflow.FlowSources
/**
* A concatenate expression using the string `redirect:` or `ajaxredirect:` or `forward:` on the left.
@@ -42,6 +40,13 @@ abstract class SpringUrlRedirectSink extends DataFlow::Node { }
*/
private class SpringViewUrlRedirectSink extends SpringUrlRedirectSink {
SpringViewUrlRedirectSink() {
// Hardcoded redirect such as "redirect:login"
this.asExpr()
.(CompileTimeConstantExpr)
.getStringValue()
.indexOf(["redirect:", "ajaxredirect:", "forward:"]) = 0 and
any(SpringRequestMappingMethod sqmm).polyCalls*(this.getEnclosingCallable())
or
exists(RedirectBuilderExpr rbe |
rbe.getRightOperand() = this.asExpr() and
any(SpringRequestMappingMethod sqmm).polyCalls*(this.getEnclosingCallable())

View File

@@ -6,9 +6,9 @@
* @tags model-generator
*/
private import internal.CaptureModels
private import internal.CaptureSummaryFlow
import internal.CaptureModels
import internal.CaptureSummaryFlow
from TargetApi api, string noflow
from DataFlowTargetApi api, string noflow
where noflow = captureNoFlow(api)
select noflow order by noflow

View File

@@ -6,8 +6,8 @@
* @tags model-generator
*/
private import internal.CaptureModels
import internal.CaptureModels
from TargetApi api, string sink
from DataFlowTargetApi api, string sink
where sink = captureSink(api)
select sink order by sink

View File

@@ -6,8 +6,8 @@
* @tags model-generator
*/
private import internal.CaptureModels
import internal.CaptureModels
from TargetApi api, string source
from DataFlowTargetApi api, string source
where source = captureSource(api)
select source order by source

View File

@@ -6,9 +6,9 @@
* @tags model-generator
*/
private import internal.CaptureModels
private import internal.CaptureSummaryFlow
import internal.CaptureModels
import internal.CaptureSummaryFlow
from TargetApi api, string flow
from DataFlowTargetApi api, string flow
where flow = captureFlow(api)
select flow order by flow

View File

@@ -5,7 +5,9 @@
private import CaptureModelsSpecific
class TargetApi = TargetApiSpecific;
class DataFlowTargetApi extends TargetApiSpecific {
DataFlowTargetApi() { isRelevantForDataFlowModels(this) }
}
/**
* Holds if data can flow from `node1` to `node2` either via a read or a write of an intermediate field `f`.
@@ -40,7 +42,7 @@ private predicate isRelevantContent(DataFlow::Content c) {
* Gets the summary model for `api` with `input`, `output` and `kind`.
*/
bindingset[input, output, kind]
private string asSummaryModel(TargetApi api, string input, string output, string kind) {
private string asSummaryModel(TargetApiSpecific api, string input, string output, string kind) {
result =
asPartialModel(api) + input + ";" //
+ output + ";" //
@@ -48,13 +50,15 @@ private string asSummaryModel(TargetApi api, string input, string output, string
+ "generated"
}
string asNegativeSummaryModel(TargetApi api) { result = asPartialNegativeModel(api) + "generated" }
string asNegativeSummaryModel(TargetApiSpecific api) {
result = asPartialNegativeModel(api) + "generated"
}
/**
* Gets the value summary model for `api` with `input` and `output`.
*/
bindingset[input, output]
private string asValueModel(TargetApi api, string input, string output) {
string asValueModel(TargetApiSpecific api, string input, string output) {
result = asSummaryModel(api, input, output, "value")
}
@@ -62,7 +66,7 @@ private string asValueModel(TargetApi api, string input, string output) {
* Gets the taint summary model for `api` with `input` and `output`.
*/
bindingset[input, output]
private string asTaintModel(TargetApi api, string input, string output) {
private string asTaintModel(TargetApiSpecific api, string input, string output) {
result = asSummaryModel(api, input, output, "taint")
}
@@ -70,7 +74,7 @@ private string asTaintModel(TargetApi api, string input, string output) {
* Gets the sink model for `api` with `input` and `kind`.
*/
bindingset[input, kind]
private string asSinkModel(TargetApi api, string input, string kind) {
private string asSinkModel(TargetApiSpecific api, string input, string kind) {
result =
asPartialModel(api) + input + ";" //
+ kind + ";" //
@@ -81,7 +85,7 @@ private string asSinkModel(TargetApi api, string input, string kind) {
* Gets the source model for `api` with `output` and `kind`.
*/
bindingset[output, kind]
private string asSourceModel(TargetApi api, string output, string kind) {
private string asSourceModel(TargetApiSpecific api, string output, string kind) {
result =
asPartialModel(api) + output + ";" //
+ kind + ";" //
@@ -91,7 +95,7 @@ private string asSourceModel(TargetApi api, string output, string kind) {
/**
* Gets the summary model of `api`, if it follows the `fluent` programming pattern (returns `this`).
*/
string captureQualifierFlow(TargetApi api) {
string captureQualifierFlow(TargetApiSpecific api) {
exists(DataFlowImplCommon::ReturnNodeExt ret |
api = returnNodeEnclosingCallable(ret) and
isOwnInstanceAccessNode(ret)
@@ -140,7 +144,7 @@ private class ThroughFlowConfig extends TaintTracking::Configuration {
override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) {
source instanceof DataFlow::ParameterNode and
source.getEnclosingCallable() instanceof TargetApi and
source.getEnclosingCallable() instanceof DataFlowTargetApi and
state.(TaintRead).getStep() = 0
}
@@ -184,7 +188,7 @@ private class ThroughFlowConfig extends TaintTracking::Configuration {
/**
* Gets the summary model(s) of `api`, if there is flow from parameters to return value or parameter.
*/
string captureThroughFlow(TargetApi api) {
string captureThroughFlow(DataFlowTargetApi api) {
exists(
ThroughFlowConfig config, DataFlow::ParameterNode p,
DataFlowImplCommon::ReturnNodeExt returnNodeExt, string input, string output
@@ -211,7 +215,7 @@ private class FromSourceConfiguration extends TaintTracking::Configuration {
override predicate isSource(DataFlow::Node source) { ExternalFlow::sourceNode(source, _) }
override predicate isSink(DataFlow::Node sink) {
exists(TargetApi c |
exists(DataFlowTargetApi c |
sink instanceof DataFlowImplCommon::ReturnNodeExt and
sink.getEnclosingCallable() = c
)
@@ -229,11 +233,12 @@ private class FromSourceConfiguration extends TaintTracking::Configuration {
/**
* Gets the source model(s) of `api`, if there is flow from an existing known source to the return of `api`.
*/
string captureSource(TargetApi api) {
string captureSource(DataFlowTargetApi api) {
exists(DataFlow::Node source, DataFlow::Node sink, FromSourceConfiguration config, string kind |
config.hasFlow(source, sink) and
ExternalFlow::sourceNode(source, kind) and
api = sink.getEnclosingCallable() and
isRelevantSourceKind(kind) and
result = asSourceModel(api, returnNodeAsOutput(sink), kind)
)
}
@@ -258,7 +263,7 @@ private class PropagateToSinkConfiguration extends PropagateToSinkConfigurationS
/**
* Gets the sink model(s) of `api`, if there is flow from a parameter to an existing known sink.
*/
string captureSink(TargetApi api) {
string captureSink(DataFlowTargetApi api) {
exists(DataFlow::Node src, DataFlow::Node sink, PropagateToSinkConfiguration config, string kind |
config.hasFlow(src, sink) and
ExternalFlow::sinkNode(sink, kind) and

View File

@@ -55,9 +55,15 @@ private predicate isJdkInternal(J::CompilationUnit cu) {
private predicate isRelevantForModels(J::Callable api) {
not isInTestFile(api.getCompilationUnit().getFile()) and
not isJdkInternal(api.getCompilationUnit()) and
not api instanceof J::MainMethod
not api instanceof J::MainMethod and
not api instanceof J::StaticInitializer
}
/**
* Holds if it is relevant to generate models for `api` based on data flow analysis.
*/
predicate isRelevantForDataFlowModels = isRelevantForModels/1;
/**
* A class of Callables that are relevant for generating summary, source and sinks models for.
*
@@ -259,3 +265,9 @@ predicate isRelevantSinkKind(string kind) {
not kind.matches("regex-use%") and
not kind = "write-file"
}
/**
* Holds if `kind` is a relevant source kind for creating source models.
*/
bindingset[kind]
predicate isRelevantSourceKind(string kind) { any() }

View File

@@ -67,7 +67,7 @@ private import CaptureModels
* Captured Model:
* ```p;Foo;true;addToList;;Argument[0];Argument[1];taint```
*/
string captureFlow(TargetApi api) {
string captureFlow(DataFlowTargetApi api) {
result = captureQualifierFlow(api) or
result = captureThroughFlow(api)
}
@@ -76,7 +76,7 @@ string captureFlow(TargetApi api) {
* Gets the negative summary for `api`, if any.
* A negative summary is generated, if there does not exist any positive flow.
*/
string captureNoFlow(TargetApi api) {
string captureNoFlow(DataFlowTargetApi api) {
not exists(captureFlow(api)) and
result = asNegativeSummaryModel(api)
}