diff --git a/plugins/java/ArithmeticTainted.ql b/plugins/java/ArithmeticTainted.ql new file mode 100644 index 0000000..efa4168 --- /dev/null +++ b/plugins/java/ArithmeticTainted.ql @@ -0,0 +1,26 @@ + + + import java + import semmle.code.java.dataflow.DataFlow + import semmle.code.java.security.ArithmeticCommon + import semmle.code.java.security.ArithmeticTaintedQuery + + module Flow = + DataFlow::MergePathGraph; + + import Flow::PathGraph + + from Flow::PathNode source, Flow::PathNode sink, ArithExpr exp, string effect + where + ArithmeticOverflow::flowPath(source.asPathNode1(), sink.asPathNode1()) and + overflowSink(exp, sink.getNode().asExpr()) and + effect = "overflow" + or + ArithmeticUnderflow::flowPath(source.asPathNode2(), sink.asPathNode2()) and + underflowSink(exp, sink.getNode().asExpr()) and + effect = "underflow" + select source.toString(),source.getNode().getEnclosingCallable(),source.getNode().getEnclosingCallable().getFile().getAbsolutePath(), + sink.toString(),sink.getNode().getEnclosingCallable(), sink.getNode().getEnclosingCallable().getFile().getAbsolutePath(), "arithmetic tainted" + + \ No newline at end of file diff --git a/plugins/java/BeanShellInjection.ql b/plugins/java/BeanShellInjection.ql old mode 100755 new mode 100644 index a9f970f..1986c55 --- a/plugins/java/BeanShellInjection.ql +++ b/plugins/java/BeanShellInjection.ql @@ -1,63 +1,72 @@ -import java -import semmle.code.java.dataflow.FlowSources -import DataFlow::PathGraph - - -/** A call to `Interpreter.eval`. */ -class InterpreterEvalCall extends MethodAccess { - InterpreterEvalCall() { - this.getMethod().hasName("eval") and - this.getMethod().getDeclaringType().hasQualifiedName("bsh", "Interpreter") - } -} -/** A call to `BshScriptEvaluator.evaluate`. */ -class BshScriptEvaluatorEvaluateCall extends MethodAccess { - BshScriptEvaluatorEvaluateCall() { - this.getMethod().hasName("evaluate") and - this.getMethod() - .getDeclaringType() - .hasQualifiedName("org.springframework.scripting.bsh", "BshScriptEvaluator") - } -} +import java + //import BeanShellInjection + import semmle.code.java.dataflow.FlowSources + import semmle.code.java.dataflow.TaintTracking + import BeanShellInjectionFlow::PathGraph -/** A sink for BeanShell expression injection vulnerabilities. */ -class BeanShellInjectionSink extends DataFlow::Node { - BeanShellInjectionSink() { - this.asExpr() = any(InterpreterEvalCall iec).getArgument(0) or - this.asExpr() = any(BshScriptEvaluatorEvaluateCall bseec).getArgument(0) + /** A call to `Interpreter.eval`. */ +class InterpreterEvalCall extends MethodCall { + InterpreterEvalCall() { + this.getMethod().hasName("eval") and + this.getMethod().getDeclaringType().hasQualifiedName("bsh", "Interpreter") + } } -} - - - -class BeanShellInjectionConfig extends TaintTracking::Configuration { - BeanShellInjectionConfig() { this = "BeanShellInjectionConfig" } - - override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } - - override predicate isSink(DataFlow::Node sink) { sink instanceof BeanShellInjectionSink } - - override predicate isAdditionalTaintStep(DataFlow::Node prod, DataFlow::Node succ) { - exists(ClassInstanceExpr cie | - cie.getConstructedType() - .hasQualifiedName("org.springframework.scripting.support", "StaticScriptSource") and - cie.getArgument(0) = prod.asExpr() and - cie = succ.asExpr() - ) - or - exists(MethodAccess ma | - ma.getMethod().hasName("setScript") and - ma.getMethod() + + /** A call to `BshScriptEvaluator.evaluate`. */ + class BshScriptEvaluatorEvaluateCall extends MethodCall { + BshScriptEvaluatorEvaluateCall() { + this.getMethod().hasName("evaluate") and + this.getMethod() .getDeclaringType() - .hasQualifiedName("org.springframework.scripting.support", "StaticScriptSource") and - ma.getArgument(0) = prod.asExpr() and - ma.getQualifier() = succ.asExpr() - ) + .hasQualifiedName("org.springframework.scripting.bsh", "BshScriptEvaluator") + } } -} - -from DataFlow::PathNode source, DataFlow::PathNode sink, BeanShellInjectionConfig conf -where conf.hasFlowPath(source, sink) -select source.toString(),source.getNode().getEnclosingCallable(),source.getNode().getEnclosingCallable().getFile().getAbsolutePath(), - sink.toString(),sink.getNode().getEnclosingCallable(), sink.getNode().getEnclosingCallable().getFile().getAbsolutePath(), "BeanShell injection" + + /** A sink for BeanShell expression injection vulnerabilities. */ + class BeanShellInjectionSink extends DataFlow::Node { + BeanShellInjectionSink() { + this.asExpr() = any(InterpreterEvalCall iec).getArgument(0) or + this.asExpr() = any(BshScriptEvaluatorEvaluateCall bseec).getArgument(0) + } + } + + + module BeanShellInjectionConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource } + + predicate isSink(DataFlow::Node sink) { sink instanceof BeanShellInjectionSink } + + predicate isAdditionalFlowStep(DataFlow::Node prod, DataFlow::Node succ) { + exists(ClassInstanceExpr cie | + cie.getConstructedType() + .hasQualifiedName("org.springframework.scripting.support", "StaticScriptSource") and + cie.getArgument(0) = prod.asExpr() and + cie = succ.asExpr() + ) + or + exists(MethodCall ma | + ma.getMethod().hasName("setScript") and + ma.getMethod() + .getDeclaringType() + .hasQualifiedName("org.springframework.scripting.support", "StaticScriptSource") and + ma.getArgument(0) = prod.asExpr() and + ma.getQualifier() = succ.asExpr() + ) + } + } + + module BeanShellInjectionFlow = TaintTracking::Global; + + from DataFlow::Node source, DataFlow::Node sink + where BeanShellInjectionFlow::flow(source, sink) +// select sink.getNode(), source, sink, "BeanShell injection from $@.", source.getNode(), +// "this user input" + select + source.toString(), + source.getEnclosingCallable(), + source.getEnclosingCallable().getFile().getAbsolutePath(), + sink.toString(), + sink.getEnclosingCallable(), + sink.getEnclosingCallable().getFile().getAbsolutePath(), + "el script injection" \ No newline at end of file diff --git a/plugins/java/ClientSuppliedIpUsedInSecurityCheck.ql b/plugins/java/ClientSuppliedIpUsedInSecurityCheck.ql old mode 100755 new mode 100644 index e3c0db1..84b5dad --- a/plugins/java/ClientSuppliedIpUsedInSecurityCheck.ql +++ b/plugins/java/ClientSuppliedIpUsedInSecurityCheck.ql @@ -1,9 +1,10 @@ -import java -import DataFlow -import semmle.code.java.frameworks.Networking -import semmle.code.java.security.QueryInjection -import semmle.code.java.dataflow.FlowSources -import DataFlow::PathGraph + import java + import semmle.code.java.dataflow.TaintTracking + import semmle.code.java.dataflow.FlowSources + import semmle.code.java.security.Sanitizers + import semmle.code.java.security.QueryInjection + deprecated import ClientSuppliedIpUsedInSecurityCheckFlow::PathGraph + /** @@ -13,129 +14,145 @@ import DataFlow::PathGraph * For example: `ServletRequest.getHeader("X-Forwarded-For")`. */ class ClientSuppliedIpUsedInSecurityCheck extends DataFlow::Node { - ClientSuppliedIpUsedInSecurityCheck() { - exists(MethodAccess ma | - ma.getMethod().hasName("getHeader") and - ma.getArgument(0).(CompileTimeConstantExpr).getStringValue().toLowerCase() in [ - "x-forwarded-for", "x-real-ip", "proxy-client-ip", "wl-proxy-client-ip", - "http_x_forwarded_for", "http_x_forwarded", "http_x_cluster_client_ip", "http_client_ip", - "http_forwarded_for", "http_forwarded", "http_via", "remote_addr" - ] and - ma = this.asExpr() - ) + ClientSuppliedIpUsedInSecurityCheck() { + exists(MethodCall ma | + ma.getMethod().hasName("getHeader") and + ma.getArgument(0).(CompileTimeConstantExpr).getStringValue().toLowerCase() in [ + "x-forwarded-for", "x-real-ip", "proxy-client-ip", "wl-proxy-client-ip", + "http_x_forwarded_for", "http_x_forwarded", "http_x_cluster_client_ip", "http_client_ip", + "http_forwarded_for", "http_forwarded", "http_via", "remote_addr" + ] and + ma = this.asExpr() + ) + } } -} - -/** A data flow sink for ip address forgery vulnerabilities. */ -abstract class ClientSuppliedIpUsedInSecurityCheckSink extends DataFlow::Node { } - -/** - * A data flow sink for remote client ip comparison. - * - * For example: `if (!StringUtils.startsWith(ipAddr, "192.168.")){...` determine whether the client ip starts - * with `192.168.`, and the program can be deceived by forging the ip address. - */ -private class CompareSink extends ClientSuppliedIpUsedInSecurityCheckSink { - CompareSink() { - exists(MethodAccess ma | - ma.getMethod().getName() in ["equals", "equalsIgnoreCase"] and - ma.getMethod().getDeclaringType() instanceof TypeString and - ma.getMethod().getNumberOfParameters() = 1 and - ( - ma.getArgument(0) = this.asExpr() and - ma.getQualifier().(CompileTimeConstantExpr).getStringValue() instanceof PrivateHostName and - not ma.getQualifier().(CompileTimeConstantExpr).getStringValue() = "0:0:0:0:0:0:0:1" - or + + /** A data flow sink for ip address forgery vulnerabilities. */ + abstract class ClientSuppliedIpUsedInSecurityCheckSink extends DataFlow::Node { } + + /** + * A data flow sink for remote client ip comparison. + * + * For example: `if (!StringUtils.startsWith(ipAddr, "192.168.")){...` determine whether the client ip starts + * with `192.168.`, and the program can be deceived by forging the ip address. + */ + private class CompareSink extends ClientSuppliedIpUsedInSecurityCheckSink { + CompareSink() { + exists(MethodCall ma | + ma.getMethod().getName() in ["equals", "equalsIgnoreCase"] and + ma.getMethod().getDeclaringType() instanceof TypeString and + ma.getMethod().getNumberOfParameters() = 1 and + ( + ma.getArgument(0) = this.asExpr() and + ma.getQualifier().(CompileTimeConstantExpr).getStringValue() instanceof PrivateHostName and + not ma.getQualifier().(CompileTimeConstantExpr).getStringValue() = "0:0:0:0:0:0:0:1" + or + ma.getQualifier() = this.asExpr() and + ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() instanceof PrivateHostName and + not ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "0:0:0:0:0:0:0:1" + ) + ) + or + exists(MethodCall ma | + ma.getMethod().getName() in ["contains", "startsWith"] and + ma.getMethod().getDeclaringType() instanceof TypeString and + ma.getMethod().getNumberOfParameters() = 1 and ma.getQualifier() = this.asExpr() and - ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() instanceof PrivateHostName and - not ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "0:0:0:0:0:0:0:1" + ma.getAnArgument().(CompileTimeConstantExpr).getStringValue().regexpMatch(getIpAddressRegex()) // Matches IP-address-like strings + ) + or + exists(MethodCall ma | + ma.getMethod().hasName("startsWith") and + ma.getMethod() + .getDeclaringType() + .hasQualifiedName(["org.apache.commons.lang3", "org.apache.commons.lang"], "StringUtils") and + ma.getMethod().getNumberOfParameters() = 2 and + ma.getAnArgument() = this.asExpr() and + ma.getAnArgument().(CompileTimeConstantExpr).getStringValue().regexpMatch(getIpAddressRegex()) ) - ) - or - exists(MethodAccess ma | - ma.getMethod().getName() in ["contains", "startsWith"] and - ma.getMethod().getDeclaringType() instanceof TypeString and - ma.getMethod().getNumberOfParameters() = 1 and - ma.getQualifier() = this.asExpr() and - ma.getAnArgument().(CompileTimeConstantExpr).getStringValue().regexpMatch(getIpAddressRegex()) // Matches IP-address-like strings - ) - or - exists(MethodAccess ma | - ma.getMethod().hasName("startsWith") and - ma.getMethod() - .getDeclaringType() - .hasQualifiedName(["org.apache.commons.lang3", "org.apache.commons.lang"], "StringUtils") and - ma.getMethod().getNumberOfParameters() = 2 and - ma.getAnArgument() = this.asExpr() and - ma.getAnArgument().(CompileTimeConstantExpr).getStringValue().regexpMatch(getIpAddressRegex()) - ) - or - exists(MethodAccess ma | - ma.getMethod().getName() in ["equals", "equalsIgnoreCase"] and - ma.getMethod() - .getDeclaringType() - .hasQualifiedName(["org.apache.commons.lang3", "org.apache.commons.lang"], "StringUtils") and - ma.getMethod().getNumberOfParameters() = 2 and - ma.getAnArgument() = this.asExpr() and - ma.getAnArgument().(CompileTimeConstantExpr).getStringValue() instanceof PrivateHostName and - not ma.getAnArgument().(CompileTimeConstantExpr).getStringValue() = "0:0:0:0:0:0:0:1" - ) + or + exists(MethodCall ma | + ma.getMethod().getName() in ["equals", "equalsIgnoreCase"] and + ma.getMethod() + .getDeclaringType() + .hasQualifiedName(["org.apache.commons.lang3", "org.apache.commons.lang"], "StringUtils") and + ma.getMethod().getNumberOfParameters() = 2 and + ma.getAnArgument() = this.asExpr() and + ma.getAnArgument().(CompileTimeConstantExpr).getStringValue() instanceof PrivateHostName and + not ma.getAnArgument().(CompileTimeConstantExpr).getStringValue() = "0:0:0:0:0:0:0:1" + ) + } } -} - -/** A data flow sink for sql operation. */ -private class SqlOperationSink extends ClientSuppliedIpUsedInSecurityCheckSink { - SqlOperationSink() { this instanceof QueryInjectionSink } -} - -/** A method that split string. */ -class SplitMethod extends Method { - SplitMethod() { - this.getNumberOfParameters() = 1 and - this.hasQualifiedName("java.lang", "String", "split") + + /** A data flow sink for sql operation. */ + private class SqlOperationSink extends ClientSuppliedIpUsedInSecurityCheckSink { + SqlOperationSink() { this instanceof QueryInjectionSink } } -} - -string getIpAddressRegex() { - result = - "^((10\\.((1\\d{2})?|(2[0-4]\\d)?|(25[0-5])?|([1-9]\\d|[0-9])?)(\\.)?)|(192\\.168\\.)|172\\.(1[6789]|2[0-9]|3[01])\\.)((1\\d{2})?|(2[0-4]\\d)?|(25[0-5])?|([1-9]\\d|[0-9])?)(\\.)?((1\\d{2})?|(2[0-4]\\d)?|(25[0-5])?|([1-9]\\d|[0-9])?)$" -} - - - -/** - * Taint-tracking configuration tracing flow from obtaining a client ip from an HTTP header to a sensitive use. - */ -class ClientSuppliedIpUsedInSecurityCheckConfig extends TaintTracking::Configuration { - ClientSuppliedIpUsedInSecurityCheckConfig() { this = "ClientSuppliedIpUsedInSecurityCheckConfig" } - - override predicate isSource(DataFlow::Node source) { - source instanceof ClientSuppliedIpUsedInSecurityCheck + + /** A method that split string. */ + class SplitMethod extends Method { + SplitMethod() { + this.getNumberOfParameters() = 1 and + this.hasQualifiedName("java.lang", "String", "split") + } } - - override predicate isSink(DataFlow::Node sink) { - sink instanceof ClientSuppliedIpUsedInSecurityCheckSink + + string getIpAddressRegex() { + result = + "^((10\\.((1\\d{2})?|(2[0-4]\\d)?|(25[0-5])?|([1-9]\\d|[0-9])?)(\\.)?)|(192\\.168\\.)|172\\.(1[6789]|2[0-9]|3[01])\\.)((1\\d{2})?|(2[0-4]\\d)?|(25[0-5])?|([1-9]\\d|[0-9])?)(\\.)?((1\\d{2})?|(2[0-4]\\d)?|(25[0-5])?|([1-9]\\d|[0-9])?)$" } + - /** - * Splitting a header value by `,` and taking an entry other than the first is sanitizing, because - * later entries may originate from more-trustworthy intermediate proxies, not the original client. - */ - override predicate isSanitizer(DataFlow::Node node) { - exists(ArrayAccess aa, MethodAccess ma | aa.getArray() = ma | - ma.getQualifier() = node.asExpr() and - ma.getMethod() instanceof SplitMethod and - not aa.getIndexExpr().(CompileTimeConstantExpr).getIntValue() = 0 - ) - or - node.getType() instanceof PrimitiveType - or - node.getType() instanceof BoxedType - } -} -from - DataFlow::PathNode source, DataFlow::PathNode sink, ClientSuppliedIpUsedInSecurityCheckConfig conf -where conf.hasFlowPath(source, sink) -select source.toString(),source.getNode().getEnclosingCallable(),source.getNode().getEnclosingCallable().getFile().getAbsolutePath(), - sink.toString(),sink.getNode().getEnclosingCallable(), sink.getNode().getEnclosingCallable().getFile().getAbsolutePath(), "IP address spoofing might include code" \ No newline at end of file + + /** + * Taint-tracking configuration tracing flow from obtaining a client ip from an HTTP header to a sensitive use. + */ + deprecated module ClientSuppliedIpUsedInSecurityCheckConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { + source instanceof ClientSuppliedIpUsedInSecurityCheck + } + + predicate isSink(DataFlow::Node sink) { sink instanceof ClientSuppliedIpUsedInSecurityCheckSink } + + /** + * Splitting a header value by `,` and taking an entry other than the first is sanitizing, because + * later entries may originate from more-trustworthy intermediate proxies, not the original client. + */ + predicate isBarrier(DataFlow::Node node) { + exists(ArrayAccess aa, MethodCall ma | aa.getArray() = ma | + ma.getQualifier() = node.asExpr() and + ma.getMethod() instanceof SplitMethod and + not aa.getIndexExpr().(CompileTimeConstantExpr).getIntValue() = 0 + ) + or + node instanceof SimpleTypeSanitizer + } + } + + deprecated module ClientSuppliedIpUsedInSecurityCheckFlow = + TaintTracking::Global; + + deprecated query predicate problems( + DataFlow::Node sinkNode, ClientSuppliedIpUsedInSecurityCheckFlow::PathNode source, + ClientSuppliedIpUsedInSecurityCheckFlow::PathNode sink, string message1, + DataFlow::Node sourceNode, string message2 + ) { + ClientSuppliedIpUsedInSecurityCheckFlow::flowPath(source, sink) and + sinkNode = sink.getNode() and + message1 = "IP address spoofing might include code from $@." and + sourceNode = source.getNode() and + message2 = "this user input" + } + + from DataFlow::Node source, DataFlow::Node sink + where ClientSuppliedIpUsedInSecurityCheckFlow::flow(source, sink) + select + source.toString(), + source.getEnclosingCallable(), + source.getEnclosingCallable().getFile().getAbsolutePath(), + sink.toString(), + sink.getEnclosingCallable(), + sink.getEnclosingCallable().getFile().getAbsolutePath(), + "ClientSuppliedIpUsedInSecurityCheck" + \ No newline at end of file diff --git a/plugins/java/ConditionalBypass.ql b/plugins/java/ConditionalBypass.ql index 89f1b8e..e00c2cc 100644 --- a/plugins/java/ConditionalBypass.ql +++ b/plugins/java/ConditionalBypass.ql @@ -1,16 +1,24 @@ -import java -import semmle.code.java.dataflow.DataFlow -import semmle.code.java.security.ConditionalBypassQuery -import DataFlow::PathGraph -from - DataFlow::PathNode source, DataFlow::PathNode sink, MethodAccess m, Expr e, - ConditionalBypassFlowConfig conf -where - conditionControlsMethod(m, e) and - sink.getNode().asExpr() = e and - conf.hasFlowPath(source, sink) -select source.toString(),source.getNode().getEnclosingCallable(),source.getNode().getEnclosingCallable().getFile().getAbsolutePath(), - sink.toString(),sink.getNode().getEnclosingCallable(), sink.getNode().getEnclosingCallable().getFile().getAbsolutePath(), - "Sensitive method may not be executed depending on a $@, which flows from $@.", e, - "this condition", source.getNode(), "user-controlled value" \ No newline at end of file + import java + import semmle.code.java.dataflow.DataFlow + import semmle.code.java.security.ConditionalBypassQuery + import ConditionalBypassFlow::PathGraph + + from + ConditionalBypassFlow::PathNode source, ConditionalBypassFlow::PathNode sink, MethodCall m, Expr e + where + conditionControlsMethod(m, e) and + sink.getNode().asExpr() = e and + ConditionalBypassFlow::flowPath(source, sink) +// select m, source, sink, +// "Sensitive method may not be executed depending on a $@, which flows from $@.", e, +// "this condition", source.getNode(), "user-controlled value" + select + source.toString(), + source.getNode().getEnclosingCallable(), + source.getNode().getEnclosingCallable().getFile().getAbsolutePath(), + sink.toString(), + sink.getNode().getEnclosingCallable(), + sink.getNode().getEnclosingCallable().getFile().getAbsolutePath(), + "el script injection" + \ No newline at end of file diff --git a/plugins/java/ElInjection.ql b/plugins/java/ElInjection.ql index f3654f2..e77305e 100644 --- a/plugins/java/ElInjection.ql +++ b/plugins/java/ElInjection.ql @@ -5,15 +5,15 @@ import semmle.code.java.security.GroovyInjectionQuery import semmle.code.java.security.SpelInjectionQuery import semmle.code.java.security.TemplateInjectionQuery import semmle.code.java.security.XsltInjectionQuery -import DataFlow::PathGraph +import semmle.code.java.dataflow.DataFlow - -from DataFlow::PathNode source, DataFlow::PathNode sink, GroovyInjectionConfig conf1, JexlInjectionConfig conf2, MvelInjectionFlowConfig conf3,SpelInjectionConfig conf4,TemplateInjectionFlowConfig conf5, XsltInjectionFlowConfig conf6 -where conf1.hasFlowPath(source, sink) or - conf2.hasFlowPath(source, sink) or - conf3.hasFlowPath(source, sink) or - conf4.hasFlowPath(source, sink) or - conf5.hasFlowPath(source, sink) or - conf6.hasFlowPath(source, sink) -select source.toString(),source.getNode().getEnclosingCallable(),source.getNode().getEnclosingCallable().getFile().getAbsolutePath(), -sink.toString(),sink.getNode().getEnclosingCallable(), sink.getNode().getEnclosingCallable().getFile().getAbsolutePath(), "el script injection" \ No newline at end of file +from DataFlow::Node source, DataFlow::Node sink +where + GroovyInjectionFlow::flow(source, sink) or + SpelInjectionFlow::flow(source, sink) or + MvelInjectionFlow::flow(source, sink) or + JexlInjectionFlow::flow(source, sink) or + TemplateInjectionFlow::flow(source, sink) or + XsltInjectionFlow::flow(source, sink) +select source.toString(),source.getEnclosingCallable(),source.getEnclosingCallable().getFile().getAbsolutePath(), +sink.toString(),sink.getEnclosingCallable(), sink.getEnclosingCallable().getFile().getAbsolutePath(), "el script injection" \ No newline at end of file diff --git a/plugins/java/ExecRelative.ql b/plugins/java/ExecRelative.ql index 1ef5943..5955ca1 100644 --- a/plugins/java/ExecRelative.ql +++ b/plugins/java/ExecRelative.ql @@ -1,12 +1,13 @@ -import semmle.code.java.Expr -import semmle.code.java.security.RelativePaths -import semmle.code.java.security.ExternalProcess - -from ArgumentToExec argument, string command -where - ( - relativePath(argument, command) or - arrayStartingWithRelative(argument, command) - ) and - not shellBuiltin(command) -select "","","","","",argument, "Command with a relative path '" + command + "' is executed." \ No newline at end of file + import semmle.code.java.Expr + import semmle.code.java.security.RelativePaths + import semmle.code.java.security.ExternalProcess + + from ArgumentToExec argument, string command + where + ( + relativePath(argument, command) or + arrayStartingWithRelative(argument, command) + ) and + not shellBuiltin(command) + select "","","","","",argument, "Command with a relative path '" + command + "' is executed." + \ No newline at end of file diff --git a/plugins/java/ExecTainted.ql b/plugins/java/ExecTainted.ql old mode 100755 new mode 100644 index 4347f7f..1df9396 --- a/plugins/java/ExecTainted.ql +++ b/plugins/java/ExecTainted.ql @@ -1,29 +1,9 @@ import java -import semmle.code.java.dataflow.FlowSources -import semmle.code.java.security.ExternalProcess import semmle.code.java.security.CommandLineQuery -import DataFlow::PathGraph +import InputToArgumentToExecFlow::PathGraph - -/** The class `com.jcraft.jsch.ChannelExec`. */ -private class JSchChannelExec extends RefType { - JSchChannelExec() { this.hasQualifiedName("com.jcraft.jsch", "ChannelExec") } -} - -/** A method to set an OS Command for the execution. */ -private class ChannelExecSetCommandMethod extends Method, ExecCallable { - ChannelExecSetCommandMethod() { - this.hasName("setCommand") and - this.getDeclaringType() instanceof JSchChannelExec - } - - override int getAnExecutedArgument() { result = 0 } -} - - - -// This is a clone of query `java/command-line-injection` that also includes experimental sinks. -from DataFlow::PathNode source, DataFlow::PathNode sink, ArgumentToExec execArg -where execTainted(source, sink, execArg) +from + InputToArgumentToExecFlow::PathNode source, InputToArgumentToExecFlow::PathNode sink, Expr execArg +where execIsTainted(source, sink, execArg) select source.toString(),source.getNode().getEnclosingCallable(),source.getNode().getEnclosingCallable().getFile().getAbsolutePath(), sink.toString(),sink.getNode().getEnclosingCallable(), sink.getNode().getEnclosingCallable().getFile().getAbsolutePath(), "Exec Tainted" diff --git a/plugins/java/FilePathInjection.ql b/plugins/java/FilePathInjection.ql old mode 100755 new mode 100644 index 2e634a4..52af34e --- a/plugins/java/FilePathInjection.ql +++ b/plugins/java/FilePathInjection.ql @@ -1,137 +1,127 @@ import java +import semmle.code.java.dataflow.TaintTracking +import semmle.code.java.dataflow.ExternalFlow import semmle.code.java.dataflow.FlowSources -import semmle.code.java.security.PathCreation +import semmle.code.java.security.TaintedPathQuery import semmle.code.java.security.PathSanitizer -import DataFlow::PathGraph -import java -private import semmle.code.java.dataflow.ExternalFlow -private import semmle.code.java.dataflow.FlowSources - -/** The class `com.jfinal.core.Controller`. */ -class JFinalController extends RefType { - JFinalController() { this.hasQualifiedName("com.jfinal.core", "Controller") } -} - -/** The method `getSessionAttr` of `JFinalController`. */ -class GetSessionAttributeMethod extends Method { - GetSessionAttributeMethod() { - this.getName() = "getSessionAttr" and - this.getDeclaringType().getASupertype*() instanceof JFinalController - } -} +private import semmle.code.java.security.Sanitizers -/** The method `setSessionAttr` of `JFinalController`. */ -class SetSessionAttributeMethod extends Method { - SetSessionAttributeMethod() { - this.getName() = "setSessionAttr" and - this.getDeclaringType().getASupertype*() instanceof JFinalController - } -} + + deprecated private class ActivateModels extends ActiveExperimentalModels { + ActivateModels() { this = "file-path-injection" } + } -/** A request attribute getter method of `JFinalController`. */ -class GetRequestAttributeMethod extends Method { - GetRequestAttributeMethod() { - this.getName().matches("getAttr%") and - this.getDeclaringType().getASupertype*() instanceof JFinalController - } -} -/** A request attribute setter method of `JFinalController`. */ -class SetRequestAttributeMethod extends Method { - SetRequestAttributeMethod() { - this.getName() = ["set", "setAttr"] and - this.getDeclaringType().getASupertype*() instanceof JFinalController +/** The class `com.jfinal.core.Controller`. */ +class JFinalController extends RefType { + JFinalController() { this.hasQualifiedName("com.jfinal.core", "Controller") } } -} - -/** - * Value step from a setter call to a corresponding getter call relating to a - * session or request attribute. - */ -private class SetToGetAttributeStep extends AdditionalValueStep { - override predicate step(DataFlow::Node pred, DataFlow::Node succ) { - exists(MethodAccess gma, MethodAccess sma | - ( - gma.getMethod() instanceof GetSessionAttributeMethod and - sma.getMethod() instanceof SetSessionAttributeMethod - or - gma.getMethod() instanceof GetRequestAttributeMethod and - sma.getMethod() instanceof SetRequestAttributeMethod - ) and - gma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = - sma.getArgument(0).(CompileTimeConstantExpr).getStringValue() - | - pred.asExpr() = sma.getArgument(1) and - succ.asExpr() = gma - ) + + /** The method `getSessionAttr` of `JFinalController`. */ + class GetSessionAttributeMethod extends Method { + GetSessionAttributeMethod() { + this.getName() = "getSessionAttr" and + this.getDeclaringType().getASupertype*() instanceof JFinalController + } } -} - -/** Remote flow source models relating to `JFinal`. */ -private class JFinalControllerSource extends SourceModelCsv { - override predicate row(string row) { - row = - [ - "com.jfinal.core;Controller;true;getCookie" + ["", "Object", "Objects", "ToInt", "ToLong"] + - ";;;ReturnValue;remote;manual", - "com.jfinal.core;Controller;true;getFile" + ["", "s"] + ";;;ReturnValue;remote;manual", - "com.jfinal.core;Controller;true;getHeader;;;ReturnValue;remote;manual", - "com.jfinal.core;Controller;true;getKv;;;ReturnValue;remote;manual", - "com.jfinal.core;Controller;true;getPara" + - [ - "", "Map", "ToBoolean", "ToDate", "ToInt", "ToLong", "Values", "ValuesToInt", - "ValuesToLong" - ] + ";;;ReturnValue;remote;manual", - "com.jfinal.core;Controller;true;get" + ["", "Int", "Long", "Boolean", "Date"] + - ";;;ReturnValue;remote;manual" - ] + + /** The method `setSessionAttr` of `JFinalController`. */ + class SetSessionAttributeMethod extends Method { + SetSessionAttributeMethod() { + this.getName() = "setSessionAttr" and + this.getDeclaringType().getASupertype*() instanceof JFinalController + } } -} - - - -/** A complementary sanitizer that protects against path traversal using path normalization. */ -class PathNormalizeSanitizer extends MethodAccess { - PathNormalizeSanitizer() { - exists(RefType t | - t instanceof TypePath or - t.hasQualifiedName("kotlin.io", "FilesKt") - | - this.getMethod().getDeclaringType() = t and - this.getMethod().hasName("normalize") - ) - or - this.getMethod().getDeclaringType() instanceof TypeFile and - this.getMethod().hasName(["getCanonicalPath", "getCanonicalFile"]) + + /** A request attribute getter method of `JFinalController`. */ + class GetRequestAttributeMethod extends Method { + GetRequestAttributeMethod() { + this.getName().matches("getAttr%") and + this.getDeclaringType().getASupertype*() instanceof JFinalController + } } -} - -/** A node with path normalization. */ -class NormalizedPathNode extends DataFlow::Node { - NormalizedPathNode() { - TaintTracking::localExprTaint(this.asExpr(), any(PathNormalizeSanitizer ma)) + + /** A request attribute setter method of `JFinalController`. */ + class SetRequestAttributeMethod extends Method { + SetRequestAttributeMethod() { + this.getName() = ["set", "setAttr"] and + this.getDeclaringType().getASupertype*() instanceof JFinalController + } } -} - -class InjectFilePathConfig extends TaintTracking::Configuration { - InjectFilePathConfig() { this = "InjectFilePathConfig" } - - override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } - - override predicate isSink(DataFlow::Node sink) { - sink.asExpr() = any(PathCreation p).getAnInput() and - not sink instanceof NormalizedPathNode + + /** + * Value step from a setter call to a corresponding getter call relating to a + * session or request attribute. + */ + private class SetToGetAttributeStep extends AdditionalValueStep { + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { + exists(MethodCall gma, MethodCall sma | + ( + gma.getMethod() instanceof GetSessionAttributeMethod and + sma.getMethod() instanceof SetSessionAttributeMethod + or + gma.getMethod() instanceof GetRequestAttributeMethod and + sma.getMethod() instanceof SetRequestAttributeMethod + ) and + gma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = + sma.getArgument(0).(CompileTimeConstantExpr).getStringValue() + | + pred.asExpr() = sma.getArgument(1) and + succ.asExpr() = gma + ) + } } - override predicate isSanitizer(DataFlow::Node node) { - exists(Type t | t = node.getType() | t instanceof BoxedType or t instanceof PrimitiveType) - or - node instanceof PathInjectionSanitizer - } -} + + /** A complementary sanitizer that protects against path traversal using path normalization. */ + class PathNormalizeSanitizer extends MethodCall { + PathNormalizeSanitizer() { + exists(RefType t | + t instanceof TypePath or + t.hasQualifiedName("kotlin.io", "FilesKt") + | + this.getMethod().getDeclaringType() = t and + this.getMethod().hasName("normalize") + ) + or + this.getMethod().getDeclaringType() instanceof TypeFile and + this.getMethod().hasName(["getCanonicalPath", "getCanonicalFile"]) + } + } + + /** A node with path normalization. */ + class NormalizedPathNode extends DataFlow::Node { + NormalizedPathNode() { + TaintTracking::localExprTaint(this.asExpr(), any(PathNormalizeSanitizer ma)) + } + } + + module InjectFilePathConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource } + + predicate isSink(DataFlow::Node sink) { + sink instanceof TaintedPathSink and + not sink instanceof NormalizedPathNode + } + + predicate isBarrier(DataFlow::Node node) { + node instanceof SimpleTypeSanitizer + or + node instanceof PathInjectionSanitizer + } + } + + module InjectFilePathFlow = TaintTracking::Global; + + from DataFlow::Node source, DataFlow::Node sink + where + InjectFilePathFlow::flow(source, sink) + select + source.toString(), + source.getEnclosingCallable(), + source.getEnclosingCallable().getFile().getAbsolutePath(), + sink.toString(), + sink.getEnclosingCallable(), + sink.getEnclosingCallable().getFile().getAbsolutePath(), + "file path injection" -from DataFlow::PathNode source, DataFlow::PathNode sink, InjectFilePathConfig conf -where conf.hasFlowPath(source, sink) -select source.toString(),source.getNode().getEnclosingCallable(),source.getNode().getEnclosingCallable().getFile().getAbsolutePath(), - sink.toString(),sink.getNode().getEnclosingCallable(), sink.getNode().getEnclosingCallable().getFile().getAbsolutePath(), "Possiable File Injection" - diff --git a/plugins/java/HardcodedCredentialsSourceCall.ql b/plugins/java/HardcodedCredentialsSourceCall.ql index e0daeed..b8c19d3 100644 --- a/plugins/java/HardcodedCredentialsSourceCall.ql +++ b/plugins/java/HardcodedCredentialsSourceCall.ql @@ -1,11 +1,16 @@ import java import semmle.code.java.security.HardcodedCredentialsSourceCallQuery -import DataFlow::PathGraph +import HardcodedCredentialSourceCallFlow::PathGraph from - DataFlow::PathNode source, DataFlow::PathNode sink, - HardcodedCredentialSourceCallConfiguration conf -where conf.hasFlowPath(source, sink) -select source.toString(),source.getNode().getEnclosingCallable(),source.getNode().getEnclosingCallable().getFile().getAbsolutePath(), - sink.toString(),sink.getNode().getEnclosingCallable(), sink.getNode().getEnclosingCallable().getFile().getAbsolutePath(), "Hard-coded value flows to $@.", sink.getNode(), - "sensitive call" \ No newline at end of file + DataFlow::Node source, + DataFlow::Node sink +where HardcodedCredentialSourceCallFlow::flow(source, sink) +select +source.toString(), +source.getEnclosingCallable(), +source.getEnclosingCallable().getFile().getAbsolutePath(), +sink.toString(), +sink.getEnclosingCallable(), +sink.getEnclosingCallable().getFile().getAbsolutePath(), +"file path injection" diff --git a/plugins/java/HardcodedJwtKey.ql b/plugins/java/HardcodedJwtKey.ql old mode 100755 new mode 100644 index 8dd0c1d..044ebf4 --- a/plugins/java/HardcodedJwtKey.ql +++ b/plugins/java/HardcodedJwtKey.ql @@ -1,7 +1,5 @@ import java import semmle.code.java.dataflow.TaintTracking -import DataFlow::PathGraph - private import semmle.code.java.dataflow.ExternalFlow private import semmle.code.java.dataflow.FlowSources @@ -84,7 +82,7 @@ class HardcodedKeyStringSource extends JwtKeySource { */ private class SignTokenSink extends JwtTokenSink { SignTokenSink() { - exists(MethodAccess ma | + exists(MethodCall ma | ma.getMethod() instanceof SignTokenMethod and this.asExpr() = ma.getArgument(0) ) @@ -96,7 +94,7 @@ private class SignTokenSink extends JwtTokenSink { */ private class VerifyTokenSink extends JwtTokenSink { VerifyTokenSink() { - exists(MethodAccess ma | + exists(MethodCall ma | ma.getMethod() instanceof VerifyTokenMethod and this.asExpr() = ma.getQualifier() ) @@ -106,15 +104,14 @@ private class VerifyTokenSink extends JwtTokenSink { /** * A configuration depicting taint flow for checking JWT token signing vulnerabilities. */ -class HardcodedJwtKeyConfiguration extends TaintTracking::Configuration { - HardcodedJwtKeyConfiguration() { this = "Hard-coded JWT Signing Key" } - - override predicate isSource(DataFlow::Node source) { source instanceof JwtKeySource } +module HardcodedJwtKeyConfiguration implements DataFlow::ConfigSig { + + predicate isSource(DataFlow::Node source) { source instanceof JwtKeySource } - override predicate isSink(DataFlow::Node sink) { sink instanceof JwtTokenSink } + predicate isSink(DataFlow::Node sink) { sink instanceof JwtTokenSink } - override predicate isAdditionalTaintStep(DataFlow::Node prev, DataFlow::Node succ) { - exists(MethodAccess ma | + predicate isAdditionalFlowStep(DataFlow::Node prev, DataFlow::Node succ) { + exists(MethodCall ma | ( ma.getMethod() instanceof GetAlgorithmMethod or ma.getMethod() instanceof RequireMethod @@ -125,27 +122,17 @@ class HardcodedJwtKeyConfiguration extends TaintTracking::Configuration { } } -/** Taint model related to verifying JWT tokens. */ -private class VerificationFlowStep extends SummaryModelCsv { - override predicate row(string row) { - row = - [ - "com.auth0.jwt.interfaces;Verification;true;build;;;Argument[-1];ReturnValue;taint;manual", - "com.auth0.jwt.interfaces;Verification;true;" + - ["acceptLeeway", "acceptExpiresAt", "acceptNotBefore", "acceptIssuedAt", "ignoreIssuedAt"] - + ";;;Argument[-1];ReturnValue;value;manual", - "com.auth0.jwt.interfaces;Verification;true;with" + - [ - "Issuer", "Subject", "Audience", "AnyOfAudience", "ClaimPresence", "Claim", - "ArrayClaim", "JWTId" - ] + ";;;Argument[-1];ReturnValue;value;manual" - ] - } -} +module HardcodedJwtKeyFlow = TaintTracking::Global; -from DataFlow::PathNode source, DataFlow::PathNode sink, HardcodedJwtKeyConfiguration cfg -where cfg.hasFlowPath(source, sink) -select source.toString(),source.getNode().getEnclosingCallable(),source.getNode().getEnclosingCallable().getFile().getAbsolutePath(), - sink.toString(),sink.getNode().getEnclosingCallable(), sink.getNode().getEnclosingCallable().getFile().getAbsolutePath(), "HardcodedJwtKey" +from DataFlow::Node source, DataFlow::Node sink +where HardcodedJwtKeyFlow::flow(source, sink) +select +source.toString(), +source.getEnclosingCallable(), +source.getEnclosingCallable().getFile().getAbsolutePath(), +sink.toString(), +sink.getEnclosingCallable(), +sink.getEnclosingCallable().getFile().getAbsolutePath(), +"hardcoded jwt key" \ No newline at end of file diff --git a/plugins/java/HardcodedPasswordField.ql b/plugins/java/HardcodedPasswordField.ql index 0c5c4e0..74036bf 100644 --- a/plugins/java/HardcodedPasswordField.ql +++ b/plugins/java/HardcodedPasswordField.ql @@ -1,6 +1,6 @@ -import java -import semmle.code.java.security.HardcodedPasswordField - -from PasswordVariable f, CompileTimeConstantExpr e -where passwordFieldAssignedHardcodedValue(f, e) -select f,"", "", e, "", "", "Sensitive field is assigned a hard-coded" \ No newline at end of file + import java + import semmle.code.java.security.HardcodedPasswordField + + from PasswordVariable f, CompileTimeConstantExpr e + where passwordFieldAssignedHardcodedValue(f, e) + select f,"", "", e, "", "", "Sensitive field is assigned a hard-coded" \ No newline at end of file diff --git a/plugins/java/HashWithoutSalt.ql b/plugins/java/HashWithoutSalt.ql new file mode 100644 index 0000000..49125f0 --- /dev/null +++ b/plugins/java/HashWithoutSalt.ql @@ -0,0 +1,187 @@ + import java + import semmle.code.java.dataflow.TaintTracking + import HashWithoutSaltFlow::PathGraph + + /** + * Gets a regular expression for matching common names of variables + * that indicate the value being held is a password. + */ + string getPasswordRegex() { result = "(?i).*pass(wd|word|code|phrase).*" } + + /** Finds variables that hold password information judging by their names. */ + class PasswordVarExpr extends VarAccess { + PasswordVarExpr() { + exists(string name | name = this.getVariable().getName().toLowerCase() | + name.regexpMatch(getPasswordRegex()) and not name.matches("%hash%") // Exclude variable names such as `passwordHash` since their values were already hashed + ) + } + } + + /** Holds if `Expr` e is a direct or indirect operand of `ae`. */ + predicate hasAddExprAncestor(AddExpr ae, Expr e) { ae.getAnOperand+() = e } + + /** The Java class `java.security.MessageDigest`. */ + class MessageDigest extends RefType { + MessageDigest() { this.hasQualifiedName("java.security", "MessageDigest") } + } + + /** The method `digest()` declared in `java.security.MessageDigest`. */ + class MDDigestMethod extends Method { + MDDigestMethod() { + this.getDeclaringType() instanceof MessageDigest and + this.hasName("digest") + } + } + + /** The method `update()` declared in `java.security.MessageDigest`. */ + class MDUpdateMethod extends Method { + MDUpdateMethod() { + this.getDeclaringType() instanceof MessageDigest and + this.hasName("update") + } + } + + /** The hashing method that could taint the input. */ + class MDHashMethodCall extends MethodCall { + MDHashMethodCall() { + ( + this.getMethod() instanceof MDDigestMethod or + this.getMethod() instanceof MDUpdateMethod + ) and + this.getNumArgument() != 0 + } + } + + /** + * Holds if `MethodCall` ma is a method access of `MDHashMethodCall` or + * invokes a method access of `MDHashMethodCall` directly or indirectly. + */ + predicate isHashAccess(MethodCall ma) { + ma instanceof MDHashMethodCall + or + exists(MethodCall mca | + ma.getMethod().calls(mca.getMethod()) and + isHashAccess(mca) and + DataFlow::localExprFlow(ma.getMethod().getAParameter().getAnAccess(), mca.getAnArgument()) + ) + } + + /** + * Holds if there is a second method access that satisfies `isHashAccess` whose qualifier or argument + * is the same as the method call `ma` that satisfies `isHashAccess`. + */ + predicate hasAnotherHashCall(MethodCall ma) { + isHashAccess(ma) and + exists(MethodCall ma2, VarAccess va | + ma2 != ma and + isHashAccess(ma2) and + not va.getVariable().getType() instanceof PrimitiveType and + ( + ma.getQualifier() = va and + ma2.getQualifier() = va.getVariable().getAnAccess() + or + ma.getQualifier() = va and + ma2.getAnArgument() = va.getVariable().getAnAccess() + or + ma.getAnArgument() = va and + ma2.getQualifier() = va.getVariable().getAnAccess() + or + ma.getAnArgument() = va and + ma2.getAnArgument() = va.getVariable().getAnAccess() + ) + ) + } + + /** + * Holds if `MethodCall` ma is part of a call graph that satisfies `isHashAccess` + * but is not at the top of the call hierarchy. + */ + predicate hasHashAncestor(MethodCall ma) { + exists(MethodCall mpa | + mpa.getMethod().calls(ma.getMethod()) and + isHashAccess(mpa) and + DataFlow::localExprFlow(mpa.getMethod().getAParameter().getAnAccess(), ma.getAnArgument()) + ) + } + + /** Holds if `MethodCall` ma is a hashing call without a sibling node making another hashing call. */ + predicate isSingleHashMethodCall(MethodCall ma) { isHashAccess(ma) and not hasAnotherHashCall(ma) } + + /** Holds if `MethodCall` ma is a single hashing call that is not invoked by a wrapper method. */ + predicate isSink(MethodCall ma) { isSingleHashMethodCall(ma) and not hasHashAncestor(ma) } + + /** Sink of hashing calls. */ + class HashWithoutSaltSink extends DataFlow::ExprNode { + HashWithoutSaltSink() { + exists(MethodCall ma | + this.asExpr() = ma.getAnArgument() and + isSink(ma) + ) + } + } + + /** + * Taint configuration tracking flow from an expression whose name suggests it holds password data + * to a method call that generates a hash without a salt. + */ + module HashWithoutSaltConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof PasswordVarExpr } + + predicate isSink(DataFlow::Node sink) { sink instanceof HashWithoutSaltSink } + + /** + * Holds if a password is concatenated with a salt then hashed together through the call `System.arraycopy(password.getBytes(), ...)`, for example, + * `System.arraycopy(password.getBytes(), 0, allBytes, 0, password.getBytes().length);` + * `System.arraycopy(salt, 0, allBytes, password.getBytes().length, salt.length);` + * `byte[] messageDigest = md.digest(allBytes);` + * Or the password is concatenated with a salt as a string. + */ + predicate isBarrier(DataFlow::Node node) { + exists(MethodCall ma | + ma.getMethod().getDeclaringType().hasQualifiedName("java.lang", "System") and + ma.getMethod().hasName("arraycopy") and + ma.getArgument(0) = node.asExpr() + ) // System.arraycopy(password.getBytes(), ...) + or + hasAddExprAncestor(_, node.asExpr()) // password+salt + or + exists(ConditionalExpr ce | ce.getAChildExpr() = node.asExpr()) // useSalt?password+":"+salt:password + or + exists(MethodCall ma | + ma.getMethod().getDeclaringType().hasQualifiedName("java.lang", "StringBuilder") and + ma.getMethod().hasName("append") and + ma.getArgument(0) = node.asExpr() // stringBuilder.append(password).append(salt) + ) + or + exists(MethodCall ma | + ma.getQualifier().(VarAccess).getVariable().getType() instanceof Interface and + ma.getAnArgument() = node.asExpr() // Method access of interface type variables requires runtime determination thus not handled + ) + } + } + + module HashWithoutSaltFlow = TaintTracking::Global; + + deprecated query predicate problems( + HashWithoutSaltFlow::PathNode sink, HashWithoutSaltFlow::PathNode source, + HashWithoutSaltFlow::PathNode sink0, string message1, HashWithoutSaltFlow::PathNode source0, + string message2 + ) { + HashWithoutSaltFlow::flowPath(source, sink) and + sink = sink0 and + source = source0 and + message1 = "$@ is hashed without a salt." and + message2 = "The password" + } + + +from DataFlow::Node source, DataFlow::Node sink +where HashWithoutSaltFlow::flow(source, sink) +select +source.toString(), +source.getEnclosingCallable(), +source.getEnclosingCallable().getFile().getAbsolutePath(), +sink.toString(), +sink.getEnclosingCallable(), +sink.getEnclosingCallable().getFile().getAbsolutePath(), +"hash without salt" \ No newline at end of file diff --git a/plugins/java/HashWithoutSalt.ql.bak b/plugins/java/HashWithoutSalt.ql.bak deleted file mode 100755 index e15079e..0000000 --- a/plugins/java/HashWithoutSalt.ql.bak +++ /dev/null @@ -1,171 +0,0 @@ -import java -import semmle.code.java.dataflow.TaintTracking -import DataFlow::PathGraph - - -/** - * Gets a regular expression for matching common names of variables - * that indicate the value being held is a password. - */ -string getPasswordRegex() { result = "(?i).*pass(wd|word|code|phrase).*" } - -/** Finds variables that hold password information judging by their names. */ -class PasswordVarExpr extends VarAccess { - PasswordVarExpr() { - exists(string name | name = this.getVariable().getName().toLowerCase() | - name.regexpMatch(getPasswordRegex()) and not name.matches("%hash%") // Exclude variable names such as `passwordHash` since their values were already hashed - ) - } -} - -/** Holds if `Expr` e is a direct or indirect operand of `ae`. */ -predicate hasAddExprAncestor(AddExpr ae, Expr e) { ae.getAnOperand+() = e } - -/** The Java class `java.security.MessageDigest`. */ -class MessageDigest extends RefType { - MessageDigest() { this.hasQualifiedName("java.security", "MessageDigest") } -} - -/** The method `digest()` declared in `java.security.MessageDigest`. */ -class MDDigestMethod extends Method { - MDDigestMethod() { - this.getDeclaringType() instanceof MessageDigest and - this.hasName("digest") - } -} - -/** The method `update()` declared in `java.security.MessageDigest`. */ -class MDUpdateMethod extends Method { - MDUpdateMethod() { - this.getDeclaringType() instanceof MessageDigest and - this.hasName("update") - } -} - -/** The hashing method that could taint the input. */ -class MDHashMethodAccess extends MethodAccess { - MDHashMethodAccess() { - ( - this.getMethod() instanceof MDDigestMethod or - this.getMethod() instanceof MDUpdateMethod - ) and - this.getNumArgument() != 0 - } -} - -/** - * Holds if `MethodAccess` ma is a method access of `MDHashMethodAccess` or - * invokes a method access of `MDHashMethodAccess` directly or indirectly. - */ -predicate isHashAccess(MethodAccess ma) { - ma instanceof MDHashMethodAccess - or - exists(MethodAccess mca | - ma.getMethod().calls(mca.getMethod()) and - isHashAccess(mca) and - DataFlow::localExprFlow(ma.getMethod().getAParameter().getAnAccess(), mca.getAnArgument()) - ) -} - -/** - * Holds if there is a second method access that satisfies `isHashAccess` whose qualifier or argument - * is the same as the method call `ma` that satisfies `isHashAccess`. - */ -predicate hasAnotherHashCall(MethodAccess ma) { - isHashAccess(ma) and - exists(MethodAccess ma2, VarAccess va | - ma2 != ma and - isHashAccess(ma2) and - not va.getVariable().getType() instanceof PrimitiveType and - ( - ma.getQualifier() = va and - ma2.getQualifier() = va.getVariable().getAnAccess() - or - ma.getQualifier() = va and - ma2.getAnArgument() = va.getVariable().getAnAccess() - or - ma.getAnArgument() = va and - ma2.getQualifier() = va.getVariable().getAnAccess() - or - ma.getAnArgument() = va and - ma2.getAnArgument() = va.getVariable().getAnAccess() - ) - ) -} - -/** - * Holds if `MethodAccess` ma is part of a call graph that satisfies `isHashAccess` - * but is not at the top of the call hierarchy. - */ -predicate hasHashAncestor(MethodAccess ma) { - exists(MethodAccess mpa | - mpa.getMethod().calls(ma.getMethod()) and - isHashAccess(mpa) and - DataFlow::localExprFlow(mpa.getMethod().getAParameter().getAnAccess(), ma.getAnArgument()) - ) -} - -/** Holds if `MethodAccess` ma is a hashing call without a sibling node making another hashing call. */ -predicate isSingleHashMethodCall(MethodAccess ma) { - isHashAccess(ma) and not hasAnotherHashCall(ma) -} - -/** Holds if `MethodAccess` ma is a single hashing call that is not invoked by a wrapper method. */ -predicate isSink(MethodAccess ma) { isSingleHashMethodCall(ma) and not hasHashAncestor(ma) } - -/** Sink of hashing calls. */ -class HashWithoutSaltSink extends DataFlow::ExprNode { - HashWithoutSaltSink() { - exists(MethodAccess ma | - this.asExpr() = ma.getAnArgument() and - isSink(ma) - ) - } -} - -/** - * Taint configuration tracking flow from an expression whose name suggests it holds password data - * to a method call that generates a hash without a salt. - */ -class HashWithoutSaltConfiguration extends TaintTracking::Configuration { - HashWithoutSaltConfiguration() { this = "HashWithoutSaltConfiguration" } - - override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof PasswordVarExpr } - - override predicate isSink(DataFlow::Node sink) { sink instanceof HashWithoutSaltSink } - - /** - * Holds if a password is concatenated with a salt then hashed together through the call `System.arraycopy(password.getBytes(), ...)`, for example, - * `System.arraycopy(password.getBytes(), 0, allBytes, 0, password.getBytes().length);` - * `System.arraycopy(salt, 0, allBytes, password.getBytes().length, salt.length);` - * `byte[] messageDigest = md.digest(allBytes);` - * Or the password is concatenated with a salt as a string. - */ - override predicate isSanitizer(DataFlow::Node node) { - exists(MethodAccess ma | - ma.getMethod().getDeclaringType().hasQualifiedName("java.lang", "System") and - ma.getMethod().hasName("arraycopy") and - ma.getArgument(0) = node.asExpr() - ) // System.arraycopy(password.getBytes(), ...) - or - exists(AddExpr e | hasAddExprAncestor(e, node.asExpr())) // password+salt - or - exists(ConditionalExpr ce | ce.getAChildExpr() = node.asExpr()) // useSalt?password+":"+salt:password - or - exists(MethodAccess ma | - ma.getMethod().getDeclaringType().hasQualifiedName("java.lang", "StringBuilder") and - ma.getMethod().hasName("append") and - ma.getArgument(0) = node.asExpr() // stringBuilder.append(password).append(salt) - ) - or - exists(MethodAccess ma | - ma.getQualifier().(VarAccess).getVariable().getType() instanceof Interface and - ma.getAnArgument() = node.asExpr() // Method access of interface type variables requires runtime determination thus not handled - ) - } -} - -from DataFlow::PathNode source, DataFlow::PathNode sink, HashWithoutSaltConfiguration cc -where cc.hasFlowPath(source, sink) -select source.toString(),source.getNode().getEnclosingCallable(),source.getNode().getEnclosingCallable().getFile().getAbsolutePath(), - sink.toString(),sink.getNode().getEnclosingCallable(), sink.getNode().getEnclosingCallable().getFile().getAbsolutePath(), "hashed without a salt." diff --git a/plugins/java/InsecureBeanValidation.ql b/plugins/java/InsecureBeanValidation.ql index a717e37..1b6e59d 100644 --- a/plugins/java/InsecureBeanValidation.ql +++ b/plugins/java/InsecureBeanValidation.ql @@ -1,72 +1,20 @@ import java -import semmle.code.java.dataflow.TaintTracking -import semmle.code.java.dataflow.FlowSources -import DataFlow::PathGraph -private import semmle.code.java.dataflow.ExternalFlow +import semmle.code.java.security.InsecureBeanValidationQuery +import BeanValidationFlow::PathGraph -/** - * A message interpolator Type that perform Expression Language (EL) evaluations - */ -class ELMessageInterpolatorType extends RefType { - ELMessageInterpolatorType() { - this.getASourceSupertype*() - .hasQualifiedName("org.hibernate.validator.messageinterpolation", - ["ResourceBundleMessageInterpolator", "ValueFormatterMessageInterpolator"]) - } -} - -/** - * A method call that sets the application's default message interpolator. - */ -class SetMessageInterpolatorCall extends MethodAccess { - SetMessageInterpolatorCall() { - exists(Method m, RefType t | - this.getMethod() = m and - m.getDeclaringType().getASourceSupertype*() = t and - ( - t.hasQualifiedName("javax.validation", ["Configuration", "ValidatorContext"]) and - m.getName() = "messageInterpolator" - or - t.hasQualifiedName("org.springframework.validation.beanvalidation", - ["CustomValidatorBean", "LocalValidatorFactoryBean"]) and - m.getName() = "setMessageInterpolator" - ) - ) - } - - /** - * The message interpolator is likely to be safe, because it does not process Java Expression Language expressions. - */ - predicate isSafe() { not this.getAnArgument().getType() instanceof ELMessageInterpolatorType } -} - -/** - * Taint tracking BeanValidationConfiguration describing the flow of data from user input - * to the argument of a method that builds constraint error messages. - */ -class BeanValidationConfig extends TaintTracking::Configuration { - BeanValidationConfig() { this = "BeanValidationConfig" } - - override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } - - override predicate isSink(DataFlow::Node sink) { sink instanceof BeanValidationSink } -} - -/** - * A bean validation sink, such as method `buildConstraintViolationWithTemplate` - * declared on a subtype of `javax.validation.ConstraintValidatorContext`. - */ -private class BeanValidationSink extends DataFlow::Node { - BeanValidationSink() { sinkNode(this, "bean-validation") } -} - -from BeanValidationConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink +from DataFlow::Node source, DataFlow::Node sink where - ( +( not exists(SetMessageInterpolatorCall c) or exists(SetMessageInterpolatorCall c | not c.isSafe()) - ) and - cfg.hasFlowPath(source, sink) -select sink.getNode(), source, sink, "Custom constraint error message contains an unsanitized $@.", - source, "user-provided value" \ No newline at end of file +) and +BeanValidationFlow::flow(source, sink) +select +source.toString(), +source.getEnclosingCallable(), +source.getEnclosingCallable().getFile().getAbsolutePath(), +sink.toString(), +sink.getEnclosingCallable(), +sink.getEnclosingCallable().getFile().getAbsolutePath(), +"insecure bean validation" \ No newline at end of file diff --git a/plugins/java/InsecureDirectoryConfig.ql b/plugins/java/InsecureDirectoryConfig.ql old mode 100755 new mode 100644 index 2e4be58..9bd7296 --- a/plugins/java/InsecureDirectoryConfig.ql +++ b/plugins/java/InsecureDirectoryConfig.ql @@ -1,37 +1,41 @@ import java import semmle.code.xml.WebXML + + /** + * The default `` element in a `web.xml` file. + */ + private class DefaultTomcatServlet extends WebServletClass { + DefaultTomcatServlet() { + this.getTextValue() = "org.apache.catalina.servlets.DefaultServlet" //Default servlet of Tomcat and other servlet containers derived from Tomcat like Glassfish + } + } + + /** + * The `` element in a `web.xml` file, nested under a `` element controlling directory listing. + */ + class DirectoryListingInitParam extends WebXmlElement { + DirectoryListingInitParam() { + this.getName() = "init-param" and + this.getAChild("param-name").getTextValue() = "listings" and + exists(WebServlet servlet | + this.getParent() = servlet and + servlet.getAChild("servlet-class") instanceof DefaultTomcatServlet + ) + } + + /** + * Check the `` element (true - enabled, false - disabled) + */ + predicate isListingEnabled() { + this.getAChild("param-value").getTextValue().toLowerCase() = "true" + } + } + + deprecated query predicate problems(DirectoryListingInitParam initp, string message) { + initp.isListingEnabled() and + message = "Directory listing should be disabled to mitigate filename and path disclosure." + } - -/** - * The default `` element in a `web.xml` file. - */ -private class DefaultTomcatServlet extends WebServletClass { - DefaultTomcatServlet() { - this.getTextValue() = "org.apache.catalina.servlets.DefaultServlet" //Default servlet of Tomcat and other servlet containers derived from Tomcat like Glassfish - } -} - -/** - * The `` element in a `web.xml` file, nested under a `` element controlling directory listing. - */ -class DirectoryListingInitParam extends WebXmlElement { - DirectoryListingInitParam() { - this.getName() = "init-param" and - this.getAChild("param-name").getTextValue() = "listings" and - exists(WebServlet servlet | - this.getParent() = servlet and - servlet.getAChild("servlet-class") instanceof DefaultTomcatServlet - ) - } - - /** - * Check the `` element (true - enabled, false - disabled) - */ - predicate isListingEnabled() { - this.getAChild("param-value").getTextValue().toLowerCase() = "true" - } -} - -from DirectoryListingInitParam initp -where initp.isListingEnabled() -select "","","","","",initp, "Directory listing should be disabled to mitigate filename and path disclosure." + from DirectoryListingInitParam initp + where initp.isListingEnabled() + select "","","","","",initp, "Directory listing should be disabled to mitigate filename and path disclosure." \ No newline at end of file diff --git a/plugins/java/InsecureLdapAuth.ql b/plugins/java/InsecureLdapAuth.ql old mode 100755 new mode 100644 index 2edbd24..f4f4763 --- a/plugins/java/InsecureLdapAuth.ql +++ b/plugins/java/InsecureLdapAuth.ql @@ -1,200 +1,16 @@ -import java -import DataFlow -import semmle.code.java.frameworks.Jndi -import semmle.code.java.frameworks.Networking -import semmle.code.java.dataflow.TaintTracking -import DataFlow::PathGraph -/** - * Insecure (non-SSL, non-private) LDAP URL string literal. - */ -class InsecureLdapUrlLiteral extends StringLiteral { - InsecureLdapUrlLiteral() { - // Match connection strings with the LDAP protocol and without private IP addresses to reduce false positives. - exists(string s | this.getValue() = s | - s.regexpMatch("(?i)ldap://[\\[a-zA-Z0-9].*") and - not s.substring(7, s.length()) instanceof PrivateHostName - ) - } -} - -/** The class `java.util.Hashtable`. */ -class TypeHashtable extends Class { - TypeHashtable() { this.getSourceDeclaration().hasQualifiedName("java.util", "Hashtable") } -} - -/** - * Holds if a non-private LDAP string is concatenated from both protocol and host. - */ -predicate concatInsecureLdapString(Expr protocol, Expr host) { - protocol.(CompileTimeConstantExpr).getStringValue() = "ldap://" and - not exists(string hostString | - hostString = host.(CompileTimeConstantExpr).getStringValue() or - hostString = - host.(VarAccess).getVariable().getAnAssignedValue().(CompileTimeConstantExpr).getStringValue() - | - hostString.length() = 0 or // Empty host is loopback address - hostString instanceof PrivateHostName - ) -} - -/** Gets the leftmost operand in a concatenated string */ -Expr getLeftmostConcatOperand(Expr expr) { - if expr instanceof AddExpr - then result = getLeftmostConcatOperand(expr.(AddExpr).getLeftOperand()) - else result = expr -} - -/** - * String concatenated with `InsecureLdapUrlLiteral`. - */ -class InsecureLdapUrl extends Expr { - InsecureLdapUrl() { - this instanceof InsecureLdapUrlLiteral - or - concatInsecureLdapString(this.(AddExpr).getLeftOperand(), - getLeftmostConcatOperand(this.(AddExpr).getRightOperand())) - } -} - -/** - * Holds if `ma` writes the `java.naming.provider.url` (also known as `Context.PROVIDER_URL`) key of a `Hashtable`. - */ -predicate isProviderUrlSetter(MethodAccess ma) { - ma.getMethod().getDeclaringType().getAnAncestor() instanceof TypeHashtable and - ma.getMethod().hasName(["put", "setProperty"]) and - ( - ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "java.naming.provider.url" - or - exists(Field f | - ma.getArgument(0) = f.getAnAccess() and - f.hasName("PROVIDER_URL") and - f.getDeclaringType() instanceof TypeNamingContext - ) - ) -} - -/** - * Holds if `ma` sets `fieldValue` to `envValue` in some `Hashtable`. - */ -bindingset[fieldValue, envValue] -predicate hasFieldValueEnv(MethodAccess ma, string fieldValue, string envValue) { - // environment.put("java.naming.security.authentication", "simple") - ma.getMethod().getDeclaringType().getAnAncestor() instanceof TypeHashtable and - ma.getMethod().hasName(["put", "setProperty"]) and - ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = fieldValue and - ma.getArgument(1).(CompileTimeConstantExpr).getStringValue() = envValue -} - -/** - * Holds if `ma` sets attribute name `fieldName` to `envValue` in some `Hashtable`. - */ -bindingset[fieldName, envValue] -predicate hasFieldNameEnv(MethodAccess ma, string fieldName, string envValue) { - // environment.put(Context.SECURITY_AUTHENTICATION, "simple") - ma.getMethod().getDeclaringType().getAnAncestor() instanceof TypeHashtable and - ma.getMethod().hasName(["put", "setProperty"]) and - exists(Field f | - ma.getArgument(0) = f.getAnAccess() and - f.hasName(fieldName) and - f.getDeclaringType() instanceof TypeNamingContext - ) and - ma.getArgument(1).(CompileTimeConstantExpr).getStringValue() = envValue -} - -/** - * Holds if `ma` sets `java.naming.security.authentication` (also known as `Context.SECURITY_AUTHENTICATION`) to `simple` in some `Hashtable`. - */ -predicate isBasicAuthEnv(MethodAccess ma) { - hasFieldValueEnv(ma, "java.naming.security.authentication", "simple") or - hasFieldNameEnv(ma, "SECURITY_AUTHENTICATION", "simple") -} - -/** - * Holds if `ma` sets `java.naming.security.protocol` (also known as `Context.SECURITY_PROTOCOL`) to `ssl` in some `Hashtable`. - */ -predicate isSslEnv(MethodAccess ma) { - hasFieldValueEnv(ma, "java.naming.security.protocol", "ssl") or - hasFieldNameEnv(ma, "SECURITY_PROTOCOL", "ssl") -} - -/** - * A taint-tracking configuration for `ldap://` URL in LDAP authentication. - */ -class InsecureUrlFlowConfig extends TaintTracking::Configuration { - InsecureUrlFlowConfig() { this = "InsecureLdapAuth:InsecureUrlFlowConfig" } - - /** Source of `ldap://` connection string. */ - override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof InsecureLdapUrl } - - /** Sink of directory context creation. */ - override predicate isSink(DataFlow::Node sink) { - exists(ConstructorCall cc | - cc.getConstructedType().getAnAncestor() instanceof TypeDirContext and - sink.asExpr() = cc.getArgument(0) - ) - } - - /** Method call of `env.put()`. */ - override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { - exists(MethodAccess ma | - pred.asExpr() = ma.getArgument(1) and - isProviderUrlSetter(ma) and - succ.asExpr() = ma.getQualifier() - ) - } -} - -/** - * A taint-tracking configuration for `simple` basic-authentication in LDAP configuration. - */ -class BasicAuthFlowConfig extends DataFlow::Configuration { - BasicAuthFlowConfig() { this = "InsecureLdapAuth:BasicAuthFlowConfig" } - - /** Source of `simple` configuration. */ - override predicate isSource(DataFlow::Node src) { - exists(MethodAccess ma | - isBasicAuthEnv(ma) and ma.getQualifier() = src.(PostUpdateNode).getPreUpdateNode().asExpr() - ) - } - - /** Sink of directory context creation. */ - override predicate isSink(DataFlow::Node sink) { - exists(ConstructorCall cc | - cc.getConstructedType().getAnAncestor() instanceof TypeDirContext and - sink.asExpr() = cc.getArgument(0) - ) - } -} - -/** - * A taint-tracking configuration for `ssl` configuration in LDAP authentication. - */ -class SslFlowConfig extends DataFlow::Configuration { - SslFlowConfig() { this = "InsecureLdapAuth:SSLFlowConfig" } - - /** Source of `ssl` configuration. */ - override predicate isSource(DataFlow::Node src) { - exists(MethodAccess ma | - isSslEnv(ma) and ma.getQualifier() = src.(PostUpdateNode).getPreUpdateNode().asExpr() - ) - } - - /** Sink of directory context creation. */ - override predicate isSink(DataFlow::Node sink) { - exists(ConstructorCall cc | - cc.getConstructedType().getAnAncestor() instanceof TypeDirContext and - sink.asExpr() = cc.getArgument(0) - ) - } -} - -from DataFlow::PathNode source, DataFlow::PathNode sink, InsecureUrlFlowConfig config -where - config.hasFlowPath(source, sink) and - exists(BasicAuthFlowConfig bc | bc.hasFlowTo(sink.getNode())) and - not exists(SslFlowConfig sc | sc.hasFlowTo(sink.getNode())) -select source.toString(),source.getNode().getEnclosingCallable(),source.getNode().getEnclosingCallable().getFile().getAbsolutePath(), - sink.toString(),sink.getNode().getEnclosingCallable(), sink.getNode().getEnclosingCallable().getFile().getAbsolutePath(), - "LDAP connection string" + import java + import semmle.code.java.security.InsecureLdapAuthQuery + import InsecureLdapUrlFlow::PathGraph + + from InsecureLdapUrlFlow::PathNode source, InsecureLdapUrlFlow::PathNode sink + where + InsecureLdapUrlFlow::flowPath(source, sink) and + BasicAuthFlow::flowTo(sink.getNode()) and + not RequiresSslFlow::flowTo(sink.getNode()) +// select sink.getNode(), source, sink, "Insecure LDAP authentication from $@.", source.getNode(), +// "LDAP connection string" + select source.toString(),source.getNode().getEnclosingCallable(),source.getNode().getEnclosingCallable().getFile().getAbsolutePath(), + sink.toString(),sink.getNode().getEnclosingCallable(), sink.getNode().getEnclosingCallable().getFile().getAbsolutePath(), +"LDAP connection string" \ No newline at end of file diff --git a/plugins/java/InsecureRmiJmxEnvironmentConfiguration.ql b/plugins/java/InsecureRmiJmxEnvironmentConfiguration.ql old mode 100755 new mode 100644 index 6659696..269378f --- a/plugins/java/InsecureRmiJmxEnvironmentConfiguration.ql +++ b/plugins/java/InsecureRmiJmxEnvironmentConfiguration.ql @@ -1,80 +1,100 @@ -import java -import semmle.code.java.dataflow.DataFlow -import semmle.code.java.Maps - - -/** Holds if `constructor` instantiates an RMI or JMX server. */ -predicate isRmiOrJmxServerCreateConstructor(Constructor constructor) { - constructor - .getDeclaringType() - .hasQualifiedName("javax.management.remote.rmi", "RMIConnectorServer") -} - -/** Holds if `method` creates an RMI or JMX server. */ -predicate isRmiOrJmxServerCreateMethod(Method method) { - method.getName() = "newJMXConnectorServer" and - method.getDeclaringType().hasQualifiedName("javax.management.remote", "JMXConnectorServerFactory") -} - /** - * Models flow from the qualifier of a - * `map.put("jmx.remote.rmi.server.credential.types", value)` call - * to an RMI or JMX initialisation call. + * @name InsecureRmiJmxAuthenticationEnvironment + * @description This query detects if a JMX/RMI server is created with a potentially dangerous environment, which could lead to code execution through insecure deserialization. + * @kind problem + * @problem.severity error + * @tags security + * experimental + * external/cwe/cwe-665 + * @precision high + * @id java/insecure-rmi-jmx-server-initialization */ -class SafeFlow extends DataFlow::Configuration { - SafeFlow() { this = "MapToPutCredentialstypeConfiguration" } - - override predicate isSource(DataFlow::Node source) { putsCredentialtypesKey(source.asExpr()) } - - override predicate isSink(DataFlow::Node sink) { - exists(Call c | - isRmiOrJmxServerCreateConstructor(c.getCallee()) or - isRmiOrJmxServerCreateMethod(c.getCallee()) - | - sink.asExpr() = c.getArgument(1) - ) - } - - /** - * Holds if a `put` call on `qualifier` puts a key match - * into the map. - */ - private predicate putsCredentialtypesKey(Expr qualifier) { - exists(MapPutCall put | - put.getKey().(CompileTimeConstantExpr).getStringValue() = - [ - "jmx.remote.rmi.server.credential.types", - "jmx.remote.rmi.server.credentials.filter.pattern" - ] - or - put.getKey() - .(FieldAccess) - .getField() - .hasQualifiedName("javax.management.remote.rmi", "RMIConnectorServer", - ["CREDENTIAL_TYPES", "CREDENTIALS_FILTER_PATTERN"]) - | - put.getQualifier() = qualifier and - put.getMethod().(MapMethod).getReceiverKeyType() instanceof TypeString and - put.getMethod().(MapMethod).getReceiverValueType() instanceof TypeObject - ) - } -} -/** Gets a string describing why the application is vulnerable, depending on if the vulnerability is present due to a) a null environment b) an insecurely set environment map */ -string getRmiResult(Expr e) { - // We got a Map so we have a source and a sink node - if e instanceof NullLiteral - then - result = - "RMI/JMX server initialized with a null environment. Missing type restriction in RMI authentication method exposes the application to deserialization attacks." - else - result = - "RMI/JMX server initialized with insecure environment $@, which never restricts accepted client objects to 'java.lang.String'. This exposes to deserialization attacks against the RMI authentication method." -} + import java + import semmle.code.java.dataflow.DataFlow + import semmle.code.java.Maps + + /** Holds if `constructor` instantiates an RMI or JMX server. */ + predicate isRmiOrJmxServerCreateConstructor(Constructor constructor) { + constructor + .getDeclaringType() + .hasQualifiedName("javax.management.remote.rmi", "RMIConnectorServer") + } + + /** Holds if `method` creates an RMI or JMX server. */ + predicate isRmiOrJmxServerCreateMethod(Method method) { + method.getName() = "newJMXConnectorServer" and + method.getDeclaringType().hasQualifiedName("javax.management.remote", "JMXConnectorServerFactory") + } + + /** + * Models flow from the qualifier of a + * `map.put("jmx.remote.rmi.server.credential.types", value)` call + * to an RMI or JMX initialisation call. + */ + module SafeFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { putsCredentialtypesKey(source.asExpr()) } + + predicate isSink(DataFlow::Node sink) { + exists(Call c | + isRmiOrJmxServerCreateConstructor(c.getCallee()) or + isRmiOrJmxServerCreateMethod(c.getCallee()) + | + sink.asExpr() = c.getArgument(1) + ) + } + + /** + * Holds if a `put` call on `qualifier` puts a key match + * into the map. + */ + private predicate putsCredentialtypesKey(Expr qualifier) { + exists(MapPutCall put | + put.getKey().(CompileTimeConstantExpr).getStringValue() = + [ + "jmx.remote.rmi.server.credential.types", + "jmx.remote.rmi.server.credentials.filter.pattern" + ] + or + put.getKey() + .(FieldAccess) + .getField() + .hasQualifiedName("javax.management.remote.rmi", "RMIConnectorServer", + ["CREDENTIAL_TYPES", "CREDENTIALS_FILTER_PATTERN"]) + | + put.getQualifier() = qualifier and + put.getMethod().(MapMethod).getReceiverKeyType() instanceof TypeString and + put.getMethod().(MapMethod).getReceiverValueType() instanceof TypeObject + ) + } + } + + module SafeFlow = DataFlow::Global; + + /** Gets a string describing why the application is vulnerable, depending on if the vulnerability is present due to a) a null environment b) an insecurely set environment map */ + string getRmiResult(Expr e) { + // We got a Map so we have a source and a sink node + if e instanceof NullLiteral + then + result = + "RMI/JMX server initialized with a null environment. Missing type restriction in RMI authentication method exposes the application to deserialization attacks." + else + result = + "RMI/JMX server initialized with insecure environment $@, which never restricts accepted client objects to 'java.lang.String'. This exposes to deserialization attacks against the RMI authentication method." + } + + deprecated query predicate problems(Call c, string message1, Expr envArg, string message2) { + (isRmiOrJmxServerCreateConstructor(c.getCallee()) or isRmiOrJmxServerCreateMethod(c.getCallee())) and + envArg = c.getArgument(1) and + not SafeFlow::flowToExpr(envArg) and + message1 = getRmiResult(envArg) and + message2 = envArg.toString() + } + from Call c, Expr envArg where (isRmiOrJmxServerCreateConstructor(c.getCallee()) or isRmiOrJmxServerCreateMethod(c.getCallee())) and envArg = c.getArgument(1) and - not any(SafeFlow conf).hasFlowToExpr(envArg) -select "","","","","",c, getRmiResult(envArg), envArg, envArg.toString() + not SafeFlow::flowToExpr(envArg) +select "","","","","",c, getRmiResult(envArg), envArg, envArg.toString() \ No newline at end of file diff --git a/plugins/java/InsecureSpringActuatorConfig.ql b/plugins/java/InsecureSpringActuatorConfig.ql old mode 100755 new mode 100644 diff --git a/plugins/java/InsecureTomcatConfig.ql b/plugins/java/InsecureTomcatConfig.ql old mode 100755 new mode 100644 diff --git a/plugins/java/JShellInjection.ql b/plugins/java/JShellInjection.ql deleted file mode 100755 index 7f317d4..0000000 --- a/plugins/java/JShellInjection.ql +++ /dev/null @@ -1,80 +0,0 @@ -import java -import semmle.code.java.dataflow.FlowSources -import DataFlow::PathGraph - - -/** A sink for JShell expression injection vulnerabilities. */ -class JShellInjectionSink extends DataFlow::Node { - JShellInjectionSink() { - this.asExpr() = any(JShellEvalCall jsec).getArgument(0) - or - this.asExpr() = any(SourceCodeAnalysisWrappersCall scawc).getArgument(0) - } -} - -/** A call to `JShell.eval`. */ -private class JShellEvalCall extends MethodAccess { - JShellEvalCall() { - this.getMethod().hasName("eval") and - this.getMethod().getDeclaringType().hasQualifiedName("jdk.jshell", "JShell") and - this.getMethod().getNumberOfParameters() = 1 - } -} - -/** A call to `SourceCodeAnalysis.wrappers`. */ -private class SourceCodeAnalysisWrappersCall extends MethodAccess { - SourceCodeAnalysisWrappersCall() { - this.getMethod().hasName("wrappers") and - this.getMethod().getDeclaringType().hasQualifiedName("jdk.jshell", "SourceCodeAnalysis") and - this.getMethod().getNumberOfParameters() = 1 - } -} - -/** A call to `SourceCodeAnalysis.analyzeCompletion`. */ -class SourceCodeAnalysisAnalyzeCompletionCall extends MethodAccess { - SourceCodeAnalysisAnalyzeCompletionCall() { - this.getMethod().hasName("analyzeCompletion") and - this.getMethod() - .getDeclaringType() - .getAnAncestor() - .hasQualifiedName("jdk.jshell", "SourceCodeAnalysis") and - this.getMethod().getNumberOfParameters() = 1 - } -} - -/** A call to `CompletionInfo.source` or `CompletionInfo.remaining`. */ -class CompletionInfoSourceOrRemainingCall extends MethodAccess { - CompletionInfoSourceOrRemainingCall() { - this.getMethod().getName() in ["source", "remaining"] and - this.getMethod() - .getDeclaringType() - .getAnAncestor() - .hasQualifiedName("jdk.jshell", "SourceCodeAnalysis$CompletionInfo") and - this.getMethod().getNumberOfParameters() = 0 - } -} - - - -class JShellInjectionConfiguration extends TaintTracking::Configuration { - JShellInjectionConfiguration() { this = "JShellInjectionConfiguration" } - - override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } - - override predicate isSink(DataFlow::Node sink) { sink instanceof JShellInjectionSink } - - override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { - exists(SourceCodeAnalysisAnalyzeCompletionCall scaacc | - scaacc.getArgument(0) = pred.asExpr() and scaacc = succ.asExpr() - ) - or - exists(CompletionInfoSourceOrRemainingCall cisorc | - cisorc.getQualifier() = pred.asExpr() and cisorc = succ.asExpr() - ) - } -} - -from DataFlow::PathNode source, DataFlow::PathNode sink, JShellInjectionConfiguration conf -where conf.hasFlowPath(source, sink) -select source.toString(),source.getNode().getEnclosingCallable(),source.getNode().getEnclosingCallable().getFile().getAbsolutePath(), - sink.toString(),sink.getNode().getEnclosingCallable(), sink.getNode().getEnclosingCallable().getFile().getAbsolutePath(), "JShell injection " diff --git a/plugins/java/JakartaExpressionInjection.ql b/plugins/java/JakartaExpressionInjection.ql old mode 100755 new mode 100644 index c103c88..1720c25 --- a/plugins/java/JakartaExpressionInjection.ql +++ b/plugins/java/JakartaExpressionInjection.ql @@ -1,47 +1,44 @@ -import java + + import java + deprecated import JakartaExpressionInjectionFlow::PathGraph import semmle.code.java.dataflow.FlowSources import semmle.code.java.dataflow.TaintTracking -import DataFlow::PathGraph - -/** - * Holds if `fromNode` to `toNode` is a dataflow step that returns data from - * a bean by calling one of its getters. - */ predicate hasGetterFlow(DataFlow::Node fromNode, DataFlow::Node toNode) { - exists(MethodAccess ma, Method m | ma.getMethod() = m | - m instanceof GetterMethod and - ma.getQualifier() = fromNode.asExpr() and - ma = toNode.asExpr() - ) -} - - + exists(MethodCall ma, Method m | ma.getMethod() = m | + m instanceof GetterMethod and + ma.getQualifier() = fromNode.asExpr() and + ma = toNode.asExpr() + ) + } /** * A taint-tracking configuration for unsafe user input * that is used to construct and evaluate an expression. */ -class JakartaExpressionInjectionConfig extends TaintTracking::Configuration { - JakartaExpressionInjectionConfig() { this = "JakartaExpressionInjectionConfig" } +module JakartaExpressionInjectionConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource } - override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } + predicate isSink(DataFlow::Node sink) { sink instanceof ExpressionEvaluationSink } - override predicate isSink(DataFlow::Node sink) { sink instanceof ExpressionEvaluationSink } - - override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) { + predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) { any(TaintPropagatingCall c).taintFlow(fromNode, toNode) or hasGetterFlow(fromNode, toNode) } } +/** + * Taint-tracking flow from remote sources, through an expression, to its eventual evaluation. + */ +module JakartaExpressionInjectionFlow = TaintTracking::Global; + /** * A sink for Expresssion Language injection vulnerabilities, * i.e. method calls that run evaluation of an expression. */ private class ExpressionEvaluationSink extends DataFlow::ExprNode { ExpressionEvaluationSink() { - exists(MethodAccess ma, Method m, Expr taintFrom | + exists(MethodCall ma, Method m, Expr taintFrom | ma.getMethod() = m and taintFrom = this.asExpr() | m.getDeclaringType() instanceof ValueExpression and @@ -76,7 +73,7 @@ private class TaintPropagatingCall extends Call { TaintPropagatingCall() { taintFromExpr = this.getArgument(1) and ( - exists(Method m | this.(MethodAccess).getMethod() = m | + exists(Method m | this.(MethodCall).getMethod() = m | m.getDeclaringType() instanceof ExpressionFactory and m.hasName(["createValueExpression", "createMethodExpression"]) and taintFromExpr.getType() instanceof TypeString @@ -123,9 +120,22 @@ private class LambdaExpression extends JakartaType { } - -from DataFlow::PathNode source, DataFlow::PathNode sink, JakartaExpressionInjectionConfig conf -where conf.hasFlowPath(source, sink) -select source.toString(),source.getNode().getEnclosingCallable(),source.getNode().getEnclosingCallable().getFile().getAbsolutePath(), - sink.toString(),sink.getNode().getEnclosingCallable(), sink.getNode().getEnclosingCallable().getFile().getAbsolutePath(), "Jakarta Expression Language injection", - source.getNode(), "this user input" + deprecated query predicate problems( + DataFlow::Node sinkNode, JakartaExpressionInjectionFlow::PathNode source, + JakartaExpressionInjectionFlow::PathNode sink, string message1, DataFlow::Node sourceNode, + string message2 + ) { + JakartaExpressionInjectionFlow::flowPath(source, sink) and + sinkNode = sink.getNode() and + message1 = "Jakarta Expression Language injection from $@." and + sourceNode = source.getNode() and + message2 = "this user input" + } + + from JakartaExpressionInjectionFlow::PathNode source, JakartaExpressionInjectionFlow::PathNode sink + where + JakartaExpressionInjectionFlow::flowPath(source, sink) + select source.toString(),source.getNode().getEnclosingCallable(),source.getNode().getEnclosingCallable().getFile().getAbsolutePath(), + sink.toString(),sink.getNode().getEnclosingCallable(), sink.getNode().getEnclosingCallable().getFile().getAbsolutePath(), +"jakarta expression injection" + \ No newline at end of file diff --git a/plugins/java/JndiInjection.ql b/plugins/java/JndiInjection.ql index acff3b4..02c5574 100644 --- a/plugins/java/JndiInjection.ql +++ b/plugins/java/JndiInjection.ql @@ -1,11 +1,9 @@ import java import semmle.code.java.security.JndiInjectionQuery -import semmle.code.java.security.XsltInjectionQuery -import DataFlow::PathGraph +import JndiInjectionFlow::PathGraph -from DataFlow::PathNode source, DataFlow::PathNode sink, JndiInjectionFlowConfig conf1,XsltInjectionFlowConfig conf2 -where conf1.hasFlowPath(source, sink) or - conf2.hasFlowPath(source, sink) +from JndiInjectionFlow::PathNode source, JndiInjectionFlow::PathNode sink +where JndiInjectionFlow::flowPath(source, sink) select source.toString(),source.getNode().getEnclosingCallable(),source.getNode().getEnclosingCallable().getFile().getAbsolutePath(), -sink.toString(),sink.getNode().getEnclosingCallable(), sink.getNode().getEnclosingCallable().getFile().getAbsolutePath(), - "Maybe JNDI injection" \ No newline at end of file +sink.toString(),sink.getNode().getEnclosingCallable(), sink.getNode().getEnclosingCallable().getFile().getAbsolutePath(), +"jndi injection" \ No newline at end of file diff --git a/plugins/java/JshellInjection.ql b/plugins/java/JshellInjection.ql new file mode 100644 index 0000000..73631fa --- /dev/null +++ b/plugins/java/JshellInjection.ql @@ -0,0 +1,94 @@ +import java +import semmle.code.java.dataflow.FlowSources +import semmle.code.java.dataflow.TaintTracking +deprecated import JShellInjectionFlow::PathGraph + + +/** A sink for JShell expression injection vulnerabilities. */ +class JShellInjectionSink extends DataFlow::Node { + JShellInjectionSink() { + this.asExpr() = any(JShellEvalCall jsec).getArgument(0) + or + this.asExpr() = any(SourceCodeAnalysisWrappersCall scawc).getArgument(0) + } + } + + /** A call to `JShell.eval`. */ + private class JShellEvalCall extends MethodCall { + JShellEvalCall() { + this.getMethod().hasName("eval") and + this.getMethod().getDeclaringType().hasQualifiedName("jdk.jshell", "JShell") and + this.getMethod().getNumberOfParameters() = 1 + } + } + + /** A call to `SourceCodeAnalysis.wrappers`. */ + private class SourceCodeAnalysisWrappersCall extends MethodCall { + SourceCodeAnalysisWrappersCall() { + this.getMethod().hasName("wrappers") and + this.getMethod().getDeclaringType().hasQualifiedName("jdk.jshell", "SourceCodeAnalysis") and + this.getMethod().getNumberOfParameters() = 1 + } + } + + /** A call to `SourceCodeAnalysis.analyzeCompletion`. */ + class SourceCodeAnalysisAnalyzeCompletionCall extends MethodCall { + SourceCodeAnalysisAnalyzeCompletionCall() { + this.getMethod().hasName("analyzeCompletion") and + this.getMethod() + .getDeclaringType() + .getAnAncestor() + .hasQualifiedName("jdk.jshell", "SourceCodeAnalysis") and + this.getMethod().getNumberOfParameters() = 1 + } + } + + /** A call to `CompletionInfo.source` or `CompletionInfo.remaining`. */ + class CompletionInfoSourceOrRemainingCall extends MethodCall { + CompletionInfoSourceOrRemainingCall() { + this.getMethod().getName() in ["source", "remaining"] and + this.getMethod() + .getDeclaringType() + .getAnAncestor() + .hasQualifiedName("jdk.jshell", "SourceCodeAnalysis$CompletionInfo") and + this.getMethod().getNumberOfParameters() = 0 + } + } + + + +deprecated module JShellInjectionConfig implements DataFlow::ConfigSig { +predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource } + +predicate isSink(DataFlow::Node sink) { sink instanceof JShellInjectionSink } + +predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + exists(SourceCodeAnalysisAnalyzeCompletionCall scaacc | + scaacc.getArgument(0) = pred.asExpr() and scaacc = succ.asExpr() + ) + or + exists(CompletionInfoSourceOrRemainingCall cisorc | + cisorc.getQualifier() = pred.asExpr() and cisorc = succ.asExpr() + ) +} +} + +deprecated module JShellInjectionFlow = TaintTracking::Global; + +deprecated query predicate problems( +DataFlow::Node sinkNode, JShellInjectionFlow::PathNode source, JShellInjectionFlow::PathNode sink, +string message1, DataFlow::Node sourceNode, string message2 +) { +JShellInjectionFlow::flowPath(source, sink) and +sinkNode = sink.getNode() and +message1 = "JShell injection from $@." and +sourceNode = source.getNode() and +message2 = "this user input" +} + +from JShellInjectionFlow::PathNode source, JShellInjectionFlow::PathNode sink +where JShellInjectionFlow::flowPath(source, sink) +select source.toString(),source.getNode().getEnclosingCallable(),source.getNode().getEnclosingCallable().getFile().getAbsolutePath(), +sink.toString(),sink.getNode().getEnclosingCallable(), sink.getNode().getEnclosingCallable().getFile().getAbsolutePath(), +"jshell injection" + \ No newline at end of file diff --git a/plugins/java/JsonpInjection.ql b/plugins/java/JsonpInjection.ql old mode 100755 new mode 100644 index 81023dd..2b3e842 --- a/plugins/java/JsonpInjection.ql +++ b/plugins/java/JsonpInjection.ql @@ -1,12 +1,9 @@ -import java -import DataFlow -import semmle.code.java.dataflow.DataFlow -import semmle.code.java.dataflow.FlowSources -import DataFlow::PathGraph -import semmle.code.java.security.XSS -import semmle.code.java.dataflow.DataFlow3 -import semmle.code.java.frameworks.spring.SpringController -import semmle.code.java.deadcode.WebEntryPoints + import java + import semmle.code.java.dataflow.TaintTracking + import semmle.code.java.dataflow.FlowSources + import semmle.code.java.deadcode.WebEntryPoints + import semmle.code.java.security.XSS + deprecated import RequestResponseFlow::PathGraph /** Json string type data. */ @@ -20,7 +17,7 @@ abstract class JsonStringSource extends DataFlow::Node { } */ private class GsonString extends JsonStringSource { GsonString() { - exists(MethodAccess ma, Method m | ma.getMethod() = m | + exists(MethodCall ma, Method m | ma.getMethod() = m | m.hasName("toJson") and m.getDeclaringType().getAnAncestor().hasQualifiedName("com.google.gson", "Gson") and this.asExpr() = ma @@ -36,7 +33,7 @@ private class GsonString extends JsonStringSource { */ private class FastjsonString extends JsonStringSource { FastjsonString() { - exists(MethodAccess ma, Method m | ma.getMethod() = m | + exists(MethodCall ma, Method m | ma.getMethod() = m | m.hasName("toJSONString") and m.getDeclaringType().getAnAncestor().hasQualifiedName("com.alibaba.fastjson", "JSON") and this.asExpr() = ma @@ -52,7 +49,7 @@ private class FastjsonString extends JsonStringSource { */ private class JacksonString extends JsonStringSource { JacksonString() { - exists(MethodAccess ma, Method m | ma.getMethod() = m | + exists(MethodCall ma, Method m | ma.getMethod() = m | m.hasName("writeValueAsString") and m.getDeclaringType() .getAnAncestor() @@ -64,142 +61,157 @@ private class JacksonString extends JsonStringSource { -/** - * A method that is called to handle an HTTP GET request. - */ -abstract class RequestGetMethod extends Method { - RequestGetMethod() { - not exists(MethodAccess ma | - // Exclude apparent GET handlers that read a request entity, because this likely indicates this is not in fact a GET handler. - // This is particularly a problem with Spring handlers, which can sometimes neglect to specify a request method. - // Even if it is in fact a GET handler, such a request method will be unusable in the context `