From b12421bfc1d74668d6f21d5645e7c64c6469102d Mon Sep 17 00:00:00 2001 From: Mathew Payne <2772944+GeekMasher@users.noreply.github.com> Date: Fri, 10 Dec 2021 14:47:39 +0000 Subject: [PATCH 1/5] Add Boto3 sinks --- python/github/HardcodedSecretSinks.qll | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/python/github/HardcodedSecretSinks.qll b/python/github/HardcodedSecretSinks.qll index acb3b8819b..9c0311a2d7 100644 --- a/python/github/HardcodedSecretSinks.qll +++ b/python/github/HardcodedSecretSinks.qll @@ -150,3 +150,22 @@ class PyOtpSink extends CredentialSink { this = API::moduleImport("pyotp").getMember("TOTP").getACall().getArg(1) } } + +class Boto3Sink extends CredentialSink { + Boto3Sink() { + // https://docs.min.io/docs/how-to-use-aws-sdk-for-python-with-minio-server.html + exists(DataFlow::CallCfgNode calls | + // s3 = boto3.resource('s3', + // aws_access_key_id='YOUR-ACCESSKEYID', + // aws_secret_access_key='YOUR-SECRETACCESSKEY' + // aws_session_token="YOUR-SESSION-TOKEN" + // ) + calls = API::moduleImport("boto3").getMember(["client", "resource"]).getACall() and + ( + this = calls.getArgByName("aws_access_key_id") or + this = calls.getArgByName("aws_secret_access_key") or + this = calls.getArgByName("aws_session_token") + ) + ) + } +} From f74763bab9523a1d276cf8589cc15430cf9242e3 Mon Sep 17 00:00:00 2001 From: Mathew Payne <2772944+GeekMasher@users.noreply.github.com> Date: Fri, 10 Dec 2021 16:06:02 +0000 Subject: [PATCH 2/5] Initial query for CVE-2021-44228 --- java/CWE-094/CVE-2021-44228.ql | 49 ++++++++++++++++++++++++++++++++++ java/github/Logging.qll | 17 ++++++++---- 2 files changed, 61 insertions(+), 5 deletions(-) create mode 100644 java/CWE-094/CVE-2021-44228.ql diff --git a/java/CWE-094/CVE-2021-44228.ql b/java/CWE-094/CVE-2021-44228.ql new file mode 100644 index 0000000000..e7ef2e7f18 --- /dev/null +++ b/java/CWE-094/CVE-2021-44228.ql @@ -0,0 +1,49 @@ +/** + * @name RCE in Log4j CVE-2021-44228 + * @description Like the default query, but with custom taint steps + * @kind path-problem + * @problem.severity error + * @security-severity 6.1 + * @precision high + * @id java/custom-xss + * @tags security + * external/cwe/cwe-079 + */ + +import java +import semmle.code.java.dataflow.DataFlow +import semmle.code.java.dataflow.FlowSources +import semmle.code.java.dataflow.TaintTracking2 +import DataFlow::PathGraph +import geekmasher.Logging + +class Log4jErrors extends DataFlow::Node { + Log4jErrors() { + exists(MethodAccess ma | + ma.getMethod().getDeclaringType() instanceof Log4jLoggerType and + ( + ma.getMethod().hasName("error") or + ma.getMethod().hasName("info") or + ma.getMethod().hasName("debug") or + ma.getMethod().hasName("trace") or + ma.getMethod().hasName("debugf") or + ma.getMethod().hasName("debugv") + ) and + // this.asExpr() = ma.getArgument(0) + // Any argvs?? + this.asExpr() = ma.getAnArgument() + ) + } +} + +class Log4jConfig extends TaintTracking::Configuration { + Log4jConfig() { this = "Log4jConfig" } + + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } + + override predicate isSink(DataFlow::Node sink) { sink instanceof Log4jErrors } +} + +from DataFlow::PathNode source, DataFlow::PathNode sink, Log4jConfig conf +where conf.hasFlowPath(source, sink) +select sink.getNode(), source, sink, "You are fu...", source.getNode(), "user-provided value" diff --git a/java/github/Logging.qll b/java/github/Logging.qll index 728d5d9be2..a73fd4b87b 100644 --- a/java/github/Logging.qll +++ b/java/github/Logging.qll @@ -24,11 +24,10 @@ class PrintMethods extends LoggingMethodsSinks { // Ref :: https://github.com/github/codeql/blob/main/java/ql/src/experimental/Security/CWE/CWE-532/SensitiveInfoLog.ql // Ref :: https://github.com/github/codeql/blob/main/java/ql/src/experimental/semmle/code/java/Logging.qll#L34 -class LoggerType extends RefType { - LoggerType() { - this.hasQualifiedName("org.apache.log4j", "Category") or // Log4j 1 - this.hasQualifiedName("org.apache.logging.log4j", ["Logger", "LogBuilder"]) or // Log4j 2 - this.hasQualifiedName("org.apache.commons.logging", "Log") or +class LoggerType extends RefType { } + +class StandardLoggersType extends LoggerType { + StandardLoggersType() { // JBoss Logging (`org.jboss.logging.Logger` in some implementations like JBoss Application Server 4.0.4 did not implement `BasicLogger`) this.hasQualifiedName("org.jboss.logging", ["BasicLogger", "Logger"]) or this.hasQualifiedName("org.slf4j.spi", "LoggingEventBuilder") or @@ -41,6 +40,14 @@ class LoggerType extends RefType { } } +class Log4jLoggerType extends LoggerType { + Log4jLoggerType() { + this.hasQualifiedName("org.apache.log4j", "Category") or // Log4j 1 + this.hasQualifiedName("org.apache.logging.log4j", ["Logger", "LogBuilder"]) or // Log4j 2 + this.hasQualifiedName("org.apache.commons.logging", "Log") + } +} + class LoggingMethods extends LoggingMethodsSinks { LoggingMethods() { exists(MethodAccess ma | From 68ad59d21aa0e509bf4d7e884dd3389891e263b1 Mon Sep 17 00:00:00 2001 From: Mathew Payne <2772944+GeekMasher@users.noreply.github.com> Date: Fri, 10 Dec 2021 16:17:52 +0000 Subject: [PATCH 3/5] Fix metadata and imports --- java/CWE-094/CVE-2021-44228.ql | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/java/CWE-094/CVE-2021-44228.ql b/java/CWE-094/CVE-2021-44228.ql index e7ef2e7f18..86e98671bf 100644 --- a/java/CWE-094/CVE-2021-44228.ql +++ b/java/CWE-094/CVE-2021-44228.ql @@ -1,13 +1,13 @@ /** * @name RCE in Log4j CVE-2021-44228 - * @description Like the default query, but with custom taint steps + * @description RCE in Log4j CVE-2021-44228 * @kind path-problem * @problem.severity error - * @security-severity 6.1 + * @security-severity 9.9 * @precision high - * @id java/custom-xss + * @id java/CVE-2021-44228 * @tags security - * external/cwe/cwe-079 + * external/cwe/cwe-094 */ import java @@ -15,7 +15,8 @@ import semmle.code.java.dataflow.DataFlow import semmle.code.java.dataflow.FlowSources import semmle.code.java.dataflow.TaintTracking2 import DataFlow::PathGraph -import geekmasher.Logging + +import github.Logging class Log4jErrors extends DataFlow::Node { Log4jErrors() { @@ -29,8 +30,6 @@ class Log4jErrors extends DataFlow::Node { ma.getMethod().hasName("debugf") or ma.getMethod().hasName("debugv") ) and - // this.asExpr() = ma.getArgument(0) - // Any argvs?? this.asExpr() = ma.getAnArgument() ) } From 6c5dde764e6d3a9d227f9d5f00e500133b1d8f63 Mon Sep 17 00:00:00 2001 From: Mathew Payne <2772944+GeekMasher@users.noreply.github.com> Date: Fri, 10 Dec 2021 16:41:42 +0000 Subject: [PATCH 4/5] Move LoggerType to abstract class --- java/github/Logging.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/github/Logging.qll b/java/github/Logging.qll index a73fd4b87b..de5061b9de 100644 --- a/java/github/Logging.qll +++ b/java/github/Logging.qll @@ -24,7 +24,7 @@ class PrintMethods extends LoggingMethodsSinks { // Ref :: https://github.com/github/codeql/blob/main/java/ql/src/experimental/Security/CWE/CWE-532/SensitiveInfoLog.ql // Ref :: https://github.com/github/codeql/blob/main/java/ql/src/experimental/semmle/code/java/Logging.qll#L34 -class LoggerType extends RefType { } +abstract class LoggerType extends RefType { } class StandardLoggersType extends LoggerType { StandardLoggersType() { From 2331e8f0437086e07f9be99d63b5c4bb838e93ad Mon Sep 17 00:00:00 2001 From: Mathew Payne <2772944+GeekMasher@users.noreply.github.com> Date: Fri, 10 Dec 2021 16:51:13 +0000 Subject: [PATCH 5/5] Fix message --- java/CWE-094/CVE-2021-44228.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/CWE-094/CVE-2021-44228.ql b/java/CWE-094/CVE-2021-44228.ql index 86e98671bf..cd8568cc8d 100644 --- a/java/CWE-094/CVE-2021-44228.ql +++ b/java/CWE-094/CVE-2021-44228.ql @@ -45,4 +45,4 @@ class Log4jConfig extends TaintTracking::Configuration { from DataFlow::PathNode source, DataFlow::PathNode sink, Log4jConfig conf where conf.hasFlowPath(source, sink) -select sink.getNode(), source, sink, "You are fu...", source.getNode(), "user-provided value" +select sink.getNode(), source, sink, "Log4j sink", source.getNode(), "user-provided value"