From e0c2a437801ee49243528a9d026130ed69c259cb Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Tue, 30 Apr 2024 10:35:37 +0200 Subject: [PATCH 01/14] Java: Deprecate the content of XssLocalQuery and remove the Xss local query variant. --- .../code/java/security/XssLocalQuery.qll | 6 ++++-- .../src/Security/CWE/CWE-079/XSSLocal.qhelp | 5 ----- java/ql/src/Security/CWE/CWE-079/XSSLocal.ql | 21 ------------------- 3 files changed, 4 insertions(+), 28 deletions(-) delete mode 100644 java/ql/src/Security/CWE/CWE-079/XSSLocal.qhelp delete mode 100644 java/ql/src/Security/CWE/CWE-079/XSSLocal.ql diff --git a/java/ql/lib/semmle/code/java/security/XssLocalQuery.qll b/java/ql/lib/semmle/code/java/security/XssLocalQuery.qll index f19872bb489..5e1098865aa 100644 --- a/java/ql/lib/semmle/code/java/security/XssLocalQuery.qll +++ b/java/ql/lib/semmle/code/java/security/XssLocalQuery.qll @@ -8,7 +8,7 @@ private import semmle.code.java.security.XSS /** * A taint-tracking configuration for reasoning about cross-site scripting vulnerabilities from a local source. */ -module XssLocalConfig implements DataFlow::ConfigSig { +deprecated module XssLocalConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput } predicate isSink(DataFlow::Node sink) { sink instanceof XssSink } @@ -23,6 +23,8 @@ module XssLocalConfig implements DataFlow::ConfigSig { } /** + * DEPRECATED: Use `XssFlow` instead and configure threat model sources to include `local`. + * * Taint-tracking flow for cross-site scripting vulnerabilities from a local source. */ -module XssLocalFlow = TaintTracking::Global; +deprecated module XssLocalFlow = TaintTracking::Global; diff --git a/java/ql/src/Security/CWE/CWE-079/XSSLocal.qhelp b/java/ql/src/Security/CWE/CWE-079/XSSLocal.qhelp deleted file mode 100644 index b35c7d781ff..00000000000 --- a/java/ql/src/Security/CWE/CWE-079/XSSLocal.qhelp +++ /dev/null @@ -1,5 +0,0 @@ - - - diff --git a/java/ql/src/Security/CWE/CWE-079/XSSLocal.ql b/java/ql/src/Security/CWE/CWE-079/XSSLocal.ql deleted file mode 100644 index 09a7849fd56..00000000000 --- a/java/ql/src/Security/CWE/CWE-079/XSSLocal.ql +++ /dev/null @@ -1,21 +0,0 @@ -/** - * @name Cross-site scripting from local source - * @description Writing user input directly to a web page - * allows for a cross-site scripting vulnerability. - * @kind path-problem - * @problem.severity recommendation - * @security-severity 6.1 - * @precision medium - * @id java/xss-local - * @tags security - * external/cwe/cwe-079 - */ - -import java -import semmle.code.java.security.XssLocalQuery -import XssLocalFlow::PathGraph - -from XssLocalFlow::PathNode source, XssLocalFlow::PathNode sink -where XssLocalFlow::flowPath(source, sink) -select sink.getNode(), source, sink, "Cross-site scripting vulnerability due to $@.", - source.getNode(), "user-provided value" From 93988e5834ba51739287a9d0f390be473fcaea70 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Tue, 30 Apr 2024 10:51:13 +0200 Subject: [PATCH 02/14] Java: Deprecate the content of XxeLocalQuery and remove the Xxe local query variant. --- .../code/java/security/XxeLocalQuery.qll | 6 +++-- .../src/Security/CWE/CWE-611/XXELocal.qhelp | 5 ---- java/ql/src/Security/CWE/CWE-611/XXELocal.ql | 25 ------------------- 3 files changed, 4 insertions(+), 32 deletions(-) delete mode 100644 java/ql/src/Security/CWE/CWE-611/XXELocal.qhelp delete mode 100644 java/ql/src/Security/CWE/CWE-611/XXELocal.ql diff --git a/java/ql/lib/semmle/code/java/security/XxeLocalQuery.qll b/java/ql/lib/semmle/code/java/security/XxeLocalQuery.qll index d3ac09798a6..f6bfa8850b2 100644 --- a/java/ql/lib/semmle/code/java/security/XxeLocalQuery.qll +++ b/java/ql/lib/semmle/code/java/security/XxeLocalQuery.qll @@ -27,7 +27,7 @@ deprecated class XxeLocalConfig extends TaintTracking::Configuration { /** * A taint-tracking configuration for unvalidated local user input that is used in XML external entity expansion. */ -module XxeLocalConfig implements DataFlow::ConfigSig { +deprecated module XxeLocalConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput } predicate isSink(DataFlow::Node sink) { sink instanceof XxeSink } @@ -40,6 +40,8 @@ module XxeLocalConfig implements DataFlow::ConfigSig { } /** + * DEPRECATED: Use `XxeFlow` instead and configure threat model sources to include `local`. + * * Detect taint flow of unvalidated local user input that is used in XML external entity expansion. */ -module XxeLocalFlow = TaintTracking::Global; +deprecated module XxeLocalFlow = TaintTracking::Global; diff --git a/java/ql/src/Security/CWE/CWE-611/XXELocal.qhelp b/java/ql/src/Security/CWE/CWE-611/XXELocal.qhelp deleted file mode 100644 index 718e8437486..00000000000 --- a/java/ql/src/Security/CWE/CWE-611/XXELocal.qhelp +++ /dev/null @@ -1,5 +0,0 @@ - - - diff --git a/java/ql/src/Security/CWE/CWE-611/XXELocal.ql b/java/ql/src/Security/CWE/CWE-611/XXELocal.ql deleted file mode 100644 index 5e306a65349..00000000000 --- a/java/ql/src/Security/CWE/CWE-611/XXELocal.ql +++ /dev/null @@ -1,25 +0,0 @@ -/** - * @name Resolving XML external entity in user-controlled data from local source - * @description Parsing user-controlled XML documents and allowing expansion of external entity - * references may lead to disclosure of confidential data or denial of service. - * @kind path-problem - * @problem.severity recommendation - * @security-severity 9.1 - * @precision medium - * @id java/xxe-local - * @tags security - * external/cwe/cwe-611 - * external/cwe/cwe-776 - * external/cwe/cwe-827 - */ - -import java -import semmle.code.java.dataflow.DataFlow -import semmle.code.java.security.XxeLocalQuery -import XxeLocalFlow::PathGraph - -from XxeLocalFlow::PathNode source, XxeLocalFlow::PathNode sink -where XxeLocalFlow::flowPath(source, sink) -select sink.getNode(), source, sink, - "XML parsing depends on a $@ without guarding against external entity expansion.", - source.getNode(), "user-provided value" From 072f19008affa508b36de74ab42ef4aaa9a05398 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Tue, 30 Apr 2024 13:17:19 +0200 Subject: [PATCH 03/14] Java: Deprecate the content of ArithmeticTaintedLocalQuery and remove the arithmetic tainted local query variant. --- .../security/ArithmeticTaintedLocalQuery.qll | 14 +++++-- .../java/security/ArithmeticTaintedQuery.qll | 38 ++++++++++++++----- .../Security/CWE/CWE-190/ArithmeticTainted.ql | 8 ++-- .../CWE/CWE-190/ArithmeticTaintedLocal.qhelp | 5 --- .../CWE/CWE-190/ArithmeticTaintedLocal.ql | 38 ------------------- ...al.expected => ArithmeticTainted.expected} | 0 .../semmle/tests/ArithmeticTainted.ext.yml | 6 +++ .../semmle/tests/ArithmeticTainted.qlref | 1 + .../semmle/tests/ArithmeticTaintedLocal.qlref | 1 - 9 files changed, 50 insertions(+), 61 deletions(-) delete mode 100644 java/ql/src/Security/CWE/CWE-190/ArithmeticTaintedLocal.qhelp delete mode 100644 java/ql/src/Security/CWE/CWE-190/ArithmeticTaintedLocal.ql rename java/ql/test/query-tests/security/CWE-190/semmle/tests/{ArithmeticTaintedLocal.expected => ArithmeticTainted.expected} (100%) create mode 100644 java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.ext.yml create mode 100644 java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.qlref delete mode 100644 java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTaintedLocal.qlref diff --git a/java/ql/lib/semmle/code/java/security/ArithmeticTaintedLocalQuery.qll b/java/ql/lib/semmle/code/java/security/ArithmeticTaintedLocalQuery.qll index 979f4b23466..45311174967 100644 --- a/java/ql/lib/semmle/code/java/security/ArithmeticTaintedLocalQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ArithmeticTaintedLocalQuery.qll @@ -5,9 +5,11 @@ private import semmle.code.java.dataflow.FlowSources private import semmle.code.java.security.ArithmeticCommon /** + * DEPRECATED: Use `ArithmeticOverflowConfig` instead. + * * A taint-tracking configuration to reason about arithmetic overflow using local-user-controlled data. */ -module ArithmeticTaintedLocalOverflowConfig implements DataFlow::ConfigSig { +deprecated module ArithmeticTaintedLocalOverflowConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput } predicate isSink(DataFlow::Node sink) { overflowSink(_, sink.asExpr()) } @@ -18,15 +20,17 @@ module ArithmeticTaintedLocalOverflowConfig implements DataFlow::ConfigSig { } /** + * DEPRECATED: Use `ArithmeticOverflow` instead and configure threat model sources to include `local`. + * * Taint-tracking flow for arithmetic overflow using local-user-controlled data. */ -module ArithmeticTaintedLocalOverflowFlow = +deprecated module ArithmeticTaintedLocalOverflowFlow = TaintTracking::Global; /** * A taint-tracking configuration to reason about arithmetic underflow using local-user-controlled data. */ -module ArithmeticTaintedLocalUnderflowConfig implements DataFlow::ConfigSig { +deprecated module ArithmeticTaintedLocalUnderflowConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput } predicate isSink(DataFlow::Node sink) { underflowSink(_, sink.asExpr()) } @@ -37,7 +41,9 @@ module ArithmeticTaintedLocalUnderflowConfig implements DataFlow::ConfigSig { } /** + * DEPRECATED: Use `ArithmeticUnderflow` instead and configure threat model sources to include `local`. + * * Taint-tracking flow for arithmetic underflow using local-user-controlled data. */ -module ArithmeticTaintedLocalUnderflowFlow = +deprecated module ArithmeticTaintedLocalUnderflowFlow = TaintTracking::Global; diff --git a/java/ql/lib/semmle/code/java/security/ArithmeticTaintedQuery.qll b/java/ql/lib/semmle/code/java/security/ArithmeticTaintedQuery.qll index 5003ceb8a3a..7d58de46a67 100644 --- a/java/ql/lib/semmle/code/java/security/ArithmeticTaintedQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ArithmeticTaintedQuery.qll @@ -1,11 +1,11 @@ -/** Provides taint-tracking configurations to reason about arithmetic with unvalidated user input. */ +/** Provides taint-tracking configurations to reason about arithmetic with unvalidated input. */ import java private import semmle.code.java.dataflow.FlowSources private import semmle.code.java.security.ArithmeticCommon -/** A taint-tracking configuration to reason about overflow from unvalidated user input. */ -module RemoteUserInputOverflowConfig implements DataFlow::ConfigSig { +/** A taint-tracking configuration to reason about overflow from unvalidated input. */ +module ArithmeticOverflowConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof ThreatModelFlowSource } predicate isSink(DataFlow::Node sink) { overflowSink(_, sink.asExpr()) } @@ -15,8 +15,13 @@ module RemoteUserInputOverflowConfig implements DataFlow::ConfigSig { predicate isBarrierIn(DataFlow::Node node) { isSource(node) } } -/** A taint-tracking configuration to reason about underflow from unvalidated user input. */ -module RemoteUserInputUnderflowConfig implements DataFlow::ConfigSig { +/** + * DEPRECATED: Use `ArithmeticOverflowConfig` instead. + */ +deprecated module RemoteUserInputOverflowConfig = ArithmeticOverflowConfig; + +/** A taint-tracking configuration to reason about underflow from unvalidated input. */ +module ArithmeticUnderflowConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof ThreatModelFlowSource } predicate isSink(DataFlow::Node sink) { underflowSink(_, sink.asExpr()) } @@ -26,8 +31,23 @@ module RemoteUserInputUnderflowConfig implements DataFlow::ConfigSig { predicate isBarrierIn(DataFlow::Node node) { isSource(node) } } -/** Taint-tracking flow for overflow from unvalidated user input. */ -module RemoteUserInputOverflow = TaintTracking::Global; +/** + * DEPRECATED: Use `ArithmeticUnderflowConfig` instead. + */ +deprecated module RemoteUserInputUnderflowConfig = ArithmeticUnderflowConfig; -/** Taint-tracking flow for underflow from unvalidated user input. */ -module RemoteUserInputUnderflow = TaintTracking::Global; +/** Taint-tracking flow for overflow from unvalidated input. */ +module ArithmeticOverflow = TaintTracking::Global; + +/** + * DEPRECATED: Use `ArithmeticOverflow` instead. + */ +deprecated module RemoteUserInputOverflow = ArithmeticOverflow; + +/** Taint-tracking flow for underflow from unvalidated input. */ +module ArithmeticUnderflow = TaintTracking::Global; + +/** + * DEPRECATED: Use `ArithmeticUnderflow` instead. + */ +deprecated module RemoteUserInputUnderflow = ArithmeticUnderflow; diff --git a/java/ql/src/Security/CWE/CWE-190/ArithmeticTainted.ql b/java/ql/src/Security/CWE/CWE-190/ArithmeticTainted.ql index c32b70e30ee..1de66916f41 100644 --- a/java/ql/src/Security/CWE/CWE-190/ArithmeticTainted.ql +++ b/java/ql/src/Security/CWE/CWE-190/ArithmeticTainted.ql @@ -18,18 +18,18 @@ import semmle.code.java.security.ArithmeticCommon import semmle.code.java.security.ArithmeticTaintedQuery module Flow = - DataFlow::MergePathGraph; + DataFlow::MergePathGraph; import Flow::PathGraph from Flow::PathNode source, Flow::PathNode sink, ArithExpr exp, string effect where - RemoteUserInputOverflow::flowPath(source.asPathNode1(), sink.asPathNode1()) and + ArithmeticOverflow::flowPath(source.asPathNode1(), sink.asPathNode1()) and overflowSink(exp, sink.getNode().asExpr()) and effect = "overflow" or - RemoteUserInputUnderflow::flowPath(source.asPathNode2(), sink.asPathNode2()) and + ArithmeticUnderflow::flowPath(source.asPathNode2(), sink.asPathNode2()) and underflowSink(exp, sink.getNode().asExpr()) and effect = "underflow" select exp, source, sink, diff --git a/java/ql/src/Security/CWE/CWE-190/ArithmeticTaintedLocal.qhelp b/java/ql/src/Security/CWE/CWE-190/ArithmeticTaintedLocal.qhelp deleted file mode 100644 index 5a193f13666..00000000000 --- a/java/ql/src/Security/CWE/CWE-190/ArithmeticTaintedLocal.qhelp +++ /dev/null @@ -1,5 +0,0 @@ - - - diff --git a/java/ql/src/Security/CWE/CWE-190/ArithmeticTaintedLocal.ql b/java/ql/src/Security/CWE/CWE-190/ArithmeticTaintedLocal.ql deleted file mode 100644 index be7092ee3e0..00000000000 --- a/java/ql/src/Security/CWE/CWE-190/ArithmeticTaintedLocal.ql +++ /dev/null @@ -1,38 +0,0 @@ -/** - * @name Local-user-controlled data in arithmetic expression - * @description Arithmetic operations on user-controlled data that is not validated can cause - * overflows. - * @kind path-problem - * @problem.severity recommendation - * @security-severity 8.6 - * @precision medium - * @id java/tainted-arithmetic-local - * @tags security - * external/cwe/cwe-190 - * external/cwe/cwe-191 - */ - -import java -import semmle.code.java.dataflow.DataFlow -import semmle.code.java.security.ArithmeticCommon -import semmle.code.java.security.ArithmeticTaintedLocalQuery - -module Flow = - DataFlow::MergePathGraph; - -import Flow::PathGraph - -from Flow::PathNode source, Flow::PathNode sink, ArithExpr exp, string effect -where - ArithmeticTaintedLocalOverflowFlow::flowPath(source.asPathNode1(), sink.asPathNode1()) and - overflowSink(exp, sink.getNode().asExpr()) and - effect = "overflow" - or - ArithmeticTaintedLocalUnderflowFlow::flowPath(source.asPathNode2(), sink.asPathNode2()) and - underflowSink(exp, sink.getNode().asExpr()) and - effect = "underflow" -select exp, source, sink, - "This arithmetic expression depends on a $@, potentially causing an " + effect + ".", - source.getNode(), "user-provided value" diff --git a/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTaintedLocal.expected b/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.expected similarity index 100% rename from java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTaintedLocal.expected rename to java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.expected diff --git a/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.ext.yml b/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.ext.yml new file mode 100644 index 00000000000..63507f47738 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.ext.yml @@ -0,0 +1,6 @@ +extensions: + - addsTo: + pack: codeql/threat-models + extensible: threatModelConfiguration + data: + - ["local", true, 0] diff --git a/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.qlref b/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.qlref new file mode 100644 index 00000000000..3939653db1c --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTainted.qlref @@ -0,0 +1 @@ +Security/CWE/CWE-190/ArithmeticTainted.ql diff --git a/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTaintedLocal.qlref b/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTaintedLocal.qlref deleted file mode 100644 index 70ad2522b60..00000000000 --- a/java/ql/test/query-tests/security/CWE-190/semmle/tests/ArithmeticTaintedLocal.qlref +++ /dev/null @@ -1 +0,0 @@ -Security/CWE/CWE-190/ArithmeticTaintedLocal.ql \ No newline at end of file From 85a4dd0325104ecd613c9e3e7c25190d41906605 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Tue, 30 Apr 2024 14:04:54 +0200 Subject: [PATCH 04/14] Java: Deprecate the local content of CommandLineQuery and remove the exec tainted local query variant. --- .../automodel/src/AutomodelAlertSinkUtil.qll | 6 +--- .../code/java/security/CommandLineQuery.qll | 28 +++++++++++++------ .../src/Security/CWE/CWE-078/ExecTainted.ql | 5 ++-- .../CWE/CWE-078/ExecTaintedLocal.qhelp | 5 ---- .../Security/CWE/CWE-078/ExecTaintedLocal.ql | 27 ------------------ .../Security/CWE/CWE-078/ExecTainted.ql | 5 ++-- ...tedLocal.expected => ExecTainted.expected} | 0 .../security/CWE-078/ExecTainted.ext.yml | 6 ++++ .../security/CWE-078/ExecTainted.qlref | 1 + .../security/CWE-078/ExecTaintedLocal.qlref | 1 - 10 files changed, 31 insertions(+), 53 deletions(-) delete mode 100644 java/ql/src/Security/CWE/CWE-078/ExecTaintedLocal.qhelp delete mode 100644 java/ql/src/Security/CWE/CWE-078/ExecTaintedLocal.ql rename java/ql/test/query-tests/security/CWE-078/{ExecTaintedLocal.expected => ExecTainted.expected} (100%) create mode 100644 java/ql/test/query-tests/security/CWE-078/ExecTainted.ext.yml create mode 100644 java/ql/test/query-tests/security/CWE-078/ExecTainted.qlref delete mode 100644 java/ql/test/query-tests/security/CWE-078/ExecTaintedLocal.qlref diff --git a/java/ql/automodel/src/AutomodelAlertSinkUtil.qll b/java/ql/automodel/src/AutomodelAlertSinkUtil.qll index 8b0b19d1388..ce0305f6095 100644 --- a/java/ql/automodel/src/AutomodelAlertSinkUtil.qll +++ b/java/ql/automodel/src/AutomodelAlertSinkUtil.qll @@ -159,11 +159,7 @@ predicate sinkModelTallyPerQuery(string queryName, int alertCount, SinkModel sin SinkTallier::getSinkModelCount(alertCount, sinkModel) or queryName = "java/command-line-injection" and - exists(int c1, int c2 | - SinkTallier::getSinkModelCount(c1, sinkModel) and - SinkTallier::getSinkModelCount(c2, sinkModel) and - alertCount = c1 + c2 - ) + SinkTallier::getSinkModelCount(alertCount, sinkModel) or queryName = "java/concatenated-sql-query" and SinkTallier::getSinkModelCount(alertCount, sinkModel) diff --git a/java/ql/lib/semmle/code/java/security/CommandLineQuery.qll b/java/ql/lib/semmle/code/java/security/CommandLineQuery.qll index 7aa602bf3c7..903dae5d67e 100644 --- a/java/ql/lib/semmle/code/java/security/CommandLineQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CommandLineQuery.qll @@ -48,7 +48,7 @@ private class DefaultCommandInjectionSanitizer extends CommandInjectionSanitizer /** * A taint-tracking configuration for unvalidated user input that is used to run an external process. */ -module RemoteUserInputToArgumentToExecFlowConfig implements DataFlow::ConfigSig { +module InputToArgumentToExecFlowConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node src) { src instanceof ThreatModelFlowSource } predicate isSink(DataFlow::Node sink) { sink instanceof CommandInjectionSink } @@ -61,15 +61,24 @@ module RemoteUserInputToArgumentToExecFlowConfig implements DataFlow::ConfigSig } /** - * Taint-tracking flow for unvalidated user input that is used to run an external process. + * DEPRECATED: Use `InputToArgumentToExecFlowConfig` instead. */ -module RemoteUserInputToArgumentToExecFlow = - TaintTracking::Global; +deprecated module RemoteUserInputToArgumentToExecFlowConfig = InputToArgumentToExecFlowConfig; + +/** + * Taint-tracking flow for unvalidated input that is used to run an external process. + */ +module InputToArgumentToExecFlow = TaintTracking::Global; + +/** + * DEPRECATED: Use `InputToArgumentToExecFlow` instead. + */ +deprecated module RemoteUserInputToArgumentToExecFlow = InputToArgumentToExecFlow; /** * A taint-tracking configuration for unvalidated local user input that is used to run an external process. */ -module LocalUserInputToArgumentToExecFlowConfig implements DataFlow::ConfigSig { +deprecated module LocalUserInputToArgumentToExecFlowConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput } predicate isSink(DataFlow::Node sink) { sink instanceof CommandInjectionSink } @@ -82,9 +91,11 @@ module LocalUserInputToArgumentToExecFlowConfig implements DataFlow::ConfigSig { } /** + * DEPRECATED: Use `InputToArgumentToExecFlow` instead and configure threat model sources to include `local`. + * * Taint-tracking flow for unvalidated local user input that is used to run an external process. */ -module LocalUserInputToArgumentToExecFlow = +deprecated module LocalUserInputToArgumentToExecFlow = TaintTracking::Global; /** @@ -93,10 +104,9 @@ module LocalUserInputToArgumentToExecFlow = * reporting overlapping results. */ predicate execIsTainted( - RemoteUserInputToArgumentToExecFlow::PathNode source, - RemoteUserInputToArgumentToExecFlow::PathNode sink, Expr execArg + InputToArgumentToExecFlow::PathNode source, InputToArgumentToExecFlow::PathNode sink, Expr execArg ) { - RemoteUserInputToArgumentToExecFlow::flowPath(source, sink) and + InputToArgumentToExecFlow::flowPath(source, sink) and argumentToExec(execArg, sink.getNode()) } diff --git a/java/ql/src/Security/CWE/CWE-078/ExecTainted.ql b/java/ql/src/Security/CWE/CWE-078/ExecTainted.ql index c5f8b085878..b6f2894ad67 100644 --- a/java/ql/src/Security/CWE/CWE-078/ExecTainted.ql +++ b/java/ql/src/Security/CWE/CWE-078/ExecTainted.ql @@ -14,11 +14,10 @@ import java import semmle.code.java.security.CommandLineQuery -import RemoteUserInputToArgumentToExecFlow::PathGraph +import InputToArgumentToExecFlow::PathGraph from - RemoteUserInputToArgumentToExecFlow::PathNode source, - RemoteUserInputToArgumentToExecFlow::PathNode sink, Expr execArg + InputToArgumentToExecFlow::PathNode source, InputToArgumentToExecFlow::PathNode sink, Expr execArg where execIsTainted(source, sink, execArg) select execArg, source, sink, "This command line depends on a $@.", source.getNode(), "user-provided value" diff --git a/java/ql/src/Security/CWE/CWE-078/ExecTaintedLocal.qhelp b/java/ql/src/Security/CWE/CWE-078/ExecTaintedLocal.qhelp deleted file mode 100644 index d2f3018b0cd..00000000000 --- a/java/ql/src/Security/CWE/CWE-078/ExecTaintedLocal.qhelp +++ /dev/null @@ -1,5 +0,0 @@ - - - diff --git a/java/ql/src/Security/CWE/CWE-078/ExecTaintedLocal.ql b/java/ql/src/Security/CWE/CWE-078/ExecTaintedLocal.ql deleted file mode 100644 index 38b79c468cd..00000000000 --- a/java/ql/src/Security/CWE/CWE-078/ExecTaintedLocal.ql +++ /dev/null @@ -1,27 +0,0 @@ -/** - * @name Local-user-controlled command line - * @description Using externally controlled strings in a command line is vulnerable to malicious - * changes in the strings. - * @kind path-problem - * @problem.severity recommendation - * @security-severity 9.8 - * @precision medium - * @id java/command-line-injection-local - * @tags security - * external/cwe/cwe-078 - * external/cwe/cwe-088 - */ - -import java -import semmle.code.java.security.CommandLineQuery -import semmle.code.java.security.ExternalProcess -import LocalUserInputToArgumentToExecFlow::PathGraph - -from - LocalUserInputToArgumentToExecFlow::PathNode source, - LocalUserInputToArgumentToExecFlow::PathNode sink, Expr e -where - LocalUserInputToArgumentToExecFlow::flowPath(source, sink) and - argumentToExec(e, sink.getNode()) -select e, source, sink, "This command line depends on a $@.", source.getNode(), - "user-provided value" diff --git a/java/ql/src/experimental/Security/CWE/CWE-078/ExecTainted.ql b/java/ql/src/experimental/Security/CWE/CWE-078/ExecTainted.ql index 5d543d65011..8e3c34c0dc4 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-078/ExecTainted.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-078/ExecTainted.ql @@ -14,7 +14,7 @@ import java import semmle.code.java.security.CommandLineQuery -import RemoteUserInputToArgumentToExecFlow::PathGraph +import InputToArgumentToExecFlow::PathGraph private import semmle.code.java.dataflow.ExternalFlow private class ActivateModels extends ActiveExperimentalModels { @@ -23,8 +23,7 @@ private class ActivateModels extends ActiveExperimentalModels { // This is a clone of query `java/command-line-injection` that also includes experimental sinks. from - RemoteUserInputToArgumentToExecFlow::PathNode source, - RemoteUserInputToArgumentToExecFlow::PathNode sink, Expr execArg + InputToArgumentToExecFlow::PathNode source, InputToArgumentToExecFlow::PathNode sink, Expr execArg where execIsTainted(source, sink, execArg) select execArg, source, sink, "This command line depends on a $@.", source.getNode(), "user-provided value" diff --git a/java/ql/test/query-tests/security/CWE-078/ExecTaintedLocal.expected b/java/ql/test/query-tests/security/CWE-078/ExecTainted.expected similarity index 100% rename from java/ql/test/query-tests/security/CWE-078/ExecTaintedLocal.expected rename to java/ql/test/query-tests/security/CWE-078/ExecTainted.expected diff --git a/java/ql/test/query-tests/security/CWE-078/ExecTainted.ext.yml b/java/ql/test/query-tests/security/CWE-078/ExecTainted.ext.yml new file mode 100644 index 00000000000..63507f47738 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-078/ExecTainted.ext.yml @@ -0,0 +1,6 @@ +extensions: + - addsTo: + pack: codeql/threat-models + extensible: threatModelConfiguration + data: + - ["local", true, 0] diff --git a/java/ql/test/query-tests/security/CWE-078/ExecTainted.qlref b/java/ql/test/query-tests/security/CWE-078/ExecTainted.qlref new file mode 100644 index 00000000000..1de765a2fdf --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-078/ExecTainted.qlref @@ -0,0 +1 @@ +Security/CWE/CWE-078/ExecTainted.ql diff --git a/java/ql/test/query-tests/security/CWE-078/ExecTaintedLocal.qlref b/java/ql/test/query-tests/security/CWE-078/ExecTaintedLocal.qlref deleted file mode 100644 index 18f968747e9..00000000000 --- a/java/ql/test/query-tests/security/CWE-078/ExecTaintedLocal.qlref +++ /dev/null @@ -1 +0,0 @@ -Security/CWE/CWE-078/ExecTaintedLocal.ql From acd0fa4b7b3a1a9e3ab5cdd92f721460b2d4393d Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Tue, 30 Apr 2024 14:40:24 +0200 Subject: [PATCH 05/14] Java: Deprecate the content of ExternallyControlledFormatStringLocalQuery and remove the externally controlled format string local query variant. --- ...rnallyControlledFormatStringLocalQuery.qll | 6 +++-- ...xternallyControlledFormatStringLocal.qhelp | 5 ---- .../ExternallyControlledFormatStringLocal.ql | 25 ------------------- .../ExternallyControlledFormatString.expected | 16 ++++++++++++ .../ExternallyControlledFormatString.ext.yml | 6 +++++ ...rnallyControlledFormatStringLocal.expected | 20 --------------- ...xternallyControlledFormatStringLocal.qlref | 1 - 7 files changed, 26 insertions(+), 53 deletions(-) delete mode 100644 java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatStringLocal.qhelp delete mode 100644 java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatStringLocal.ql create mode 100644 java/ql/test/query-tests/security/CWE-134/semmle/tests/ExternallyControlledFormatString.ext.yml delete mode 100644 java/ql/test/query-tests/security/CWE-134/semmle/tests/ExternallyControlledFormatStringLocal.expected delete mode 100644 java/ql/test/query-tests/security/CWE-134/semmle/tests/ExternallyControlledFormatStringLocal.qlref diff --git a/java/ql/lib/semmle/code/java/security/ExternallyControlledFormatStringLocalQuery.qll b/java/ql/lib/semmle/code/java/security/ExternallyControlledFormatStringLocalQuery.qll index 4d07e8bddd0..482673bacc9 100644 --- a/java/ql/lib/semmle/code/java/security/ExternallyControlledFormatStringLocalQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ExternallyControlledFormatStringLocalQuery.qll @@ -5,7 +5,7 @@ private import semmle.code.java.dataflow.FlowSources private import semmle.code.java.StringFormat /** A taint-tracking configuration to reason about externally-controlled format strings from local sources. */ -module ExternallyControlledFormatStringLocalConfig implements DataFlow::ConfigSig { +deprecated module ExternallyControlledFormatStringLocalConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput } predicate isSink(DataFlow::Node sink) { @@ -18,7 +18,9 @@ module ExternallyControlledFormatStringLocalConfig implements DataFlow::ConfigSi } /** + * DEPRECATED: Use `ExternallyControlledFormatStringFlow` instead and configure threat model sources to include `local`. + * * Taint-tracking flow for externally-controlled format strings from local sources. */ -module ExternallyControlledFormatStringLocalFlow = +deprecated module ExternallyControlledFormatStringLocalFlow = TaintTracking::Global; diff --git a/java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatStringLocal.qhelp b/java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatStringLocal.qhelp deleted file mode 100644 index cb33b0d27ba..00000000000 --- a/java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatStringLocal.qhelp +++ /dev/null @@ -1,5 +0,0 @@ - - - diff --git a/java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatStringLocal.ql b/java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatStringLocal.ql deleted file mode 100644 index ef37ebac1c9..00000000000 --- a/java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatStringLocal.ql +++ /dev/null @@ -1,25 +0,0 @@ -/** - * @name Use of externally-controlled format string from local source - * @description Using external input in format strings can lead to exceptions or information leaks. - * @kind path-problem - * @problem.severity recommendation - * @security-severity 9.3 - * @precision medium - * @id java/tainted-format-string-local - * @tags security - * external/cwe/cwe-134 - */ - -import java -import semmle.code.java.StringFormat -import semmle.code.java.security.ExternallyControlledFormatStringLocalQuery -import ExternallyControlledFormatStringLocalFlow::PathGraph - -from - ExternallyControlledFormatStringLocalFlow::PathNode source, - ExternallyControlledFormatStringLocalFlow::PathNode sink, StringFormat formatCall -where - ExternallyControlledFormatStringLocalFlow::flowPath(source, sink) and - sink.getNode().asExpr() = formatCall.getFormatArgument() -select formatCall.getFormatArgument(), source, sink, "Format string depends on a $@.", - source.getNode(), "user-provided value" diff --git a/java/ql/test/query-tests/security/CWE-134/semmle/tests/ExternallyControlledFormatString.expected b/java/ql/test/query-tests/security/CWE-134/semmle/tests/ExternallyControlledFormatString.expected index 8fe019c2b2f..32cc3c6925e 100644 --- a/java/ql/test/query-tests/security/CWE-134/semmle/tests/ExternallyControlledFormatString.expected +++ b/java/ql/test/query-tests/security/CWE-134/semmle/tests/ExternallyControlledFormatString.expected @@ -1,12 +1,28 @@ edges +| Test.java:17:27:17:60 | getProperty(...) : String | Test.java:19:19:19:30 | userProperty | provenance | Src:MaD:43040 | +| Test.java:17:27:17:60 | getProperty(...) : String | Test.java:21:23:21:34 | userProperty | provenance | Src:MaD:43040 Sink:MaD:42905 | +| Test.java:17:27:17:60 | getProperty(...) : String | Test.java:23:23:23:34 | userProperty | provenance | Src:MaD:43040 Sink:MaD:42908 | +| Test.java:17:27:17:60 | getProperty(...) : String | Test.java:25:28:25:39 | userProperty | provenance | Src:MaD:43040 | +| Test.java:17:27:17:60 | getProperty(...) : String | Test.java:27:44:27:55 | userProperty | provenance | Src:MaD:43040 | | Test.java:33:30:33:74 | getParameter(...) : String | Test.java:34:20:34:32 | userParameter : String | provenance | Src:MaD:44662 | | Test.java:34:20:34:32 | userParameter : String | Test.java:37:31:37:43 | format : String | provenance | | | Test.java:37:31:37:43 | format : String | Test.java:39:25:39:30 | format | provenance | Sink:MaD:42905 | nodes +| Test.java:17:27:17:60 | getProperty(...) : String | semmle.label | getProperty(...) : String | +| Test.java:19:19:19:30 | userProperty | semmle.label | userProperty | +| Test.java:21:23:21:34 | userProperty | semmle.label | userProperty | +| Test.java:23:23:23:34 | userProperty | semmle.label | userProperty | +| Test.java:25:28:25:39 | userProperty | semmle.label | userProperty | +| Test.java:27:44:27:55 | userProperty | semmle.label | userProperty | | Test.java:33:30:33:74 | getParameter(...) : String | semmle.label | getParameter(...) : String | | Test.java:34:20:34:32 | userParameter : String | semmle.label | userParameter : String | | Test.java:37:31:37:43 | format : String | semmle.label | format : String | | Test.java:39:25:39:30 | format | semmle.label | format | subpaths #select +| Test.java:19:19:19:30 | userProperty | Test.java:17:27:17:60 | getProperty(...) : String | Test.java:19:19:19:30 | userProperty | Format string depends on a $@. | Test.java:17:27:17:60 | getProperty(...) | user-provided value | +| Test.java:21:23:21:34 | userProperty | Test.java:17:27:17:60 | getProperty(...) : String | Test.java:21:23:21:34 | userProperty | Format string depends on a $@. | Test.java:17:27:17:60 | getProperty(...) | user-provided value | +| Test.java:23:23:23:34 | userProperty | Test.java:17:27:17:60 | getProperty(...) : String | Test.java:23:23:23:34 | userProperty | Format string depends on a $@. | Test.java:17:27:17:60 | getProperty(...) | user-provided value | +| Test.java:25:28:25:39 | userProperty | Test.java:17:27:17:60 | getProperty(...) : String | Test.java:25:28:25:39 | userProperty | Format string depends on a $@. | Test.java:17:27:17:60 | getProperty(...) | user-provided value | +| Test.java:27:44:27:55 | userProperty | Test.java:17:27:17:60 | getProperty(...) : String | Test.java:27:44:27:55 | userProperty | Format string depends on a $@. | Test.java:17:27:17:60 | getProperty(...) | user-provided value | | Test.java:39:25:39:30 | format | Test.java:33:30:33:74 | getParameter(...) : String | Test.java:39:25:39:30 | format | Format string depends on a $@. | Test.java:33:30:33:74 | getParameter(...) | user-provided value | diff --git a/java/ql/test/query-tests/security/CWE-134/semmle/tests/ExternallyControlledFormatString.ext.yml b/java/ql/test/query-tests/security/CWE-134/semmle/tests/ExternallyControlledFormatString.ext.yml new file mode 100644 index 00000000000..63507f47738 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-134/semmle/tests/ExternallyControlledFormatString.ext.yml @@ -0,0 +1,6 @@ +extensions: + - addsTo: + pack: codeql/threat-models + extensible: threatModelConfiguration + data: + - ["local", true, 0] diff --git a/java/ql/test/query-tests/security/CWE-134/semmle/tests/ExternallyControlledFormatStringLocal.expected b/java/ql/test/query-tests/security/CWE-134/semmle/tests/ExternallyControlledFormatStringLocal.expected deleted file mode 100644 index 69a8314c94e..00000000000 --- a/java/ql/test/query-tests/security/CWE-134/semmle/tests/ExternallyControlledFormatStringLocal.expected +++ /dev/null @@ -1,20 +0,0 @@ -edges -| Test.java:17:27:17:60 | getProperty(...) : String | Test.java:19:19:19:30 | userProperty | provenance | Src:MaD:43040 | -| Test.java:17:27:17:60 | getProperty(...) : String | Test.java:21:23:21:34 | userProperty | provenance | Src:MaD:43040 Sink:MaD:42905 | -| Test.java:17:27:17:60 | getProperty(...) : String | Test.java:23:23:23:34 | userProperty | provenance | Src:MaD:43040 Sink:MaD:42908 | -| Test.java:17:27:17:60 | getProperty(...) : String | Test.java:25:28:25:39 | userProperty | provenance | Src:MaD:43040 | -| Test.java:17:27:17:60 | getProperty(...) : String | Test.java:27:44:27:55 | userProperty | provenance | Src:MaD:43040 | -nodes -| Test.java:17:27:17:60 | getProperty(...) : String | semmle.label | getProperty(...) : String | -| Test.java:19:19:19:30 | userProperty | semmle.label | userProperty | -| Test.java:21:23:21:34 | userProperty | semmle.label | userProperty | -| Test.java:23:23:23:34 | userProperty | semmle.label | userProperty | -| Test.java:25:28:25:39 | userProperty | semmle.label | userProperty | -| Test.java:27:44:27:55 | userProperty | semmle.label | userProperty | -subpaths -#select -| Test.java:19:19:19:30 | userProperty | Test.java:17:27:17:60 | getProperty(...) : String | Test.java:19:19:19:30 | userProperty | Format string depends on a $@. | Test.java:17:27:17:60 | getProperty(...) | user-provided value | -| Test.java:21:23:21:34 | userProperty | Test.java:17:27:17:60 | getProperty(...) : String | Test.java:21:23:21:34 | userProperty | Format string depends on a $@. | Test.java:17:27:17:60 | getProperty(...) | user-provided value | -| Test.java:23:23:23:34 | userProperty | Test.java:17:27:17:60 | getProperty(...) : String | Test.java:23:23:23:34 | userProperty | Format string depends on a $@. | Test.java:17:27:17:60 | getProperty(...) | user-provided value | -| Test.java:25:28:25:39 | userProperty | Test.java:17:27:17:60 | getProperty(...) : String | Test.java:25:28:25:39 | userProperty | Format string depends on a $@. | Test.java:17:27:17:60 | getProperty(...) | user-provided value | -| Test.java:27:44:27:55 | userProperty | Test.java:17:27:17:60 | getProperty(...) : String | Test.java:27:44:27:55 | userProperty | Format string depends on a $@. | Test.java:17:27:17:60 | getProperty(...) | user-provided value | diff --git a/java/ql/test/query-tests/security/CWE-134/semmle/tests/ExternallyControlledFormatStringLocal.qlref b/java/ql/test/query-tests/security/CWE-134/semmle/tests/ExternallyControlledFormatStringLocal.qlref deleted file mode 100644 index d8502c264e4..00000000000 --- a/java/ql/test/query-tests/security/CWE-134/semmle/tests/ExternallyControlledFormatStringLocal.qlref +++ /dev/null @@ -1 +0,0 @@ -Security/CWE/CWE-134/ExternallyControlledFormatStringLocal.ql From 301a6cc191c9a2b652ff16c21d51b8add6004a65 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Tue, 30 Apr 2024 15:28:26 +0200 Subject: [PATCH 06/14] Java: Deprecate the content of ImproperValidationOrArray and remove local query variants. --- ...alidationOfArrayConstructionLocalQuery.qll | 6 ++-- ...properValidationOfArrayIndexLocalQuery.qll | 6 ++-- ...erValidationOfArrayConstructionLocal.qhelp | 5 ---- ...roperValidationOfArrayConstructionLocal.ql | 29 ------------------- .../ImproperValidationOfArrayIndexLocal.qhelp | 5 ---- .../ImproperValidationOfArrayIndexLocal.ql | 27 ----------------- ...perValidationOfArrayConstruction.expected} | 0 ...roperValidationOfArrayConstruction.ext.yml | 6 ++++ ...mproperValidationOfArrayConstruction.qlref | 1 + ...erValidationOfArrayConstructionLocal.qlref | 1 - ...> ImproperValidationOfArrayIndex.expected} | 0 .../ImproperValidationOfArrayIndex.ext.yml | 6 ++++ .../ImproperValidationOfArrayIndex.qlref | 1 + .../ImproperValidationOfArrayIndexLocal.qlref | 1 - 14 files changed, 22 insertions(+), 72 deletions(-) delete mode 100644 java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayConstructionLocal.qhelp delete mode 100644 java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayConstructionLocal.ql delete mode 100644 java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayIndexLocal.qhelp delete mode 100644 java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayIndexLocal.ql rename java/ql/test/query-tests/security/CWE-129/semmle/tests/{ImproperValidationOfArrayConstructionLocal.expected => ImproperValidationOfArrayConstruction.expected} (100%) create mode 100644 java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayConstruction.ext.yml create mode 100644 java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayConstruction.qlref delete mode 100644 java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayConstructionLocal.qlref rename java/ql/test/query-tests/security/CWE-129/semmle/tests/{ImproperValidationOfArrayIndexLocal.expected => ImproperValidationOfArrayIndex.expected} (100%) create mode 100644 java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayIndex.ext.yml create mode 100644 java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayIndex.qlref delete mode 100644 java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayIndexLocal.qlref diff --git a/java/ql/lib/semmle/code/java/security/ImproperValidationOfArrayConstructionLocalQuery.qll b/java/ql/lib/semmle/code/java/security/ImproperValidationOfArrayConstructionLocalQuery.qll index f1d21fbfa80..1d31d7afb87 100644 --- a/java/ql/lib/semmle/code/java/security/ImproperValidationOfArrayConstructionLocalQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ImproperValidationOfArrayConstructionLocalQuery.qll @@ -7,7 +7,7 @@ private import semmle.code.java.dataflow.FlowSources /** * A taint-tracking configuration to reason about improper validation of local user-provided size used for array construction. */ -module ImproperValidationOfArrayConstructionLocalConfig implements DataFlow::ConfigSig { +deprecated module ImproperValidationOfArrayConstructionLocalConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput } predicate isSink(DataFlow::Node sink) { @@ -16,7 +16,9 @@ module ImproperValidationOfArrayConstructionLocalConfig implements DataFlow::Con } /** + * DEPRECATED: Use `ImproperValidationOfArrayConstructionFlow` instead and configure threat model sources to include `local`. + * * Taint-tracking flow for improper validation of local user-provided size used for array construction. */ -module ImproperValidationOfArrayConstructionLocalFlow = +deprecated module ImproperValidationOfArrayConstructionLocalFlow = TaintTracking::Global; diff --git a/java/ql/lib/semmle/code/java/security/ImproperValidationOfArrayIndexLocalQuery.qll b/java/ql/lib/semmle/code/java/security/ImproperValidationOfArrayIndexLocalQuery.qll index d21de6c7fdf..5f1e7c81e01 100644 --- a/java/ql/lib/semmle/code/java/security/ImproperValidationOfArrayIndexLocalQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ImproperValidationOfArrayIndexLocalQuery.qll @@ -7,7 +7,7 @@ private import semmle.code.java.dataflow.FlowSources /** * A taint-tracking configuration to reason about improper validation of local user-provided array index. */ -module ImproperValidationOfArrayIndexLocalConfig implements DataFlow::ConfigSig { +deprecated module ImproperValidationOfArrayIndexLocalConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput } predicate isSink(DataFlow::Node sink) { @@ -20,7 +20,9 @@ module ImproperValidationOfArrayIndexLocalConfig implements DataFlow::ConfigSig } /** + * DEPRECATED: Use `ImproperValidationOfArrayIndexFlow` instead and configure threat model sources to include `local`. + * * Taint-tracking flow for improper validation of local user-provided array index. */ -module ImproperValidationOfArrayIndexLocalFlow = +deprecated module ImproperValidationOfArrayIndexLocalFlow = TaintTracking::Global; diff --git a/java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayConstructionLocal.qhelp b/java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayConstructionLocal.qhelp deleted file mode 100644 index 3f467edeca3..00000000000 --- a/java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayConstructionLocal.qhelp +++ /dev/null @@ -1,5 +0,0 @@ - - - diff --git a/java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayConstructionLocal.ql b/java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayConstructionLocal.ql deleted file mode 100644 index 1ba0521ee4d..00000000000 --- a/java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayConstructionLocal.ql +++ /dev/null @@ -1,29 +0,0 @@ -/** - * @name Improper validation of local user-provided size used for array construction - * @description Using unvalidated local input as the argument to - * a construction of an array can lead to index out of bound exceptions. - * @kind path-problem - * @problem.severity recommendation - * @security-severity 8.8 - * @precision medium - * @id java/improper-validation-of-array-construction-local - * @tags security - * external/cwe/cwe-129 - */ - -import java -import semmle.code.java.security.internal.ArraySizing -import semmle.code.java.security.ImproperValidationOfArrayConstructionLocalQuery -import ImproperValidationOfArrayConstructionLocalFlow::PathGraph - -from - ImproperValidationOfArrayConstructionLocalFlow::PathNode source, - ImproperValidationOfArrayConstructionLocalFlow::PathNode sink, Expr sizeExpr, - ArrayCreationExpr arrayCreation, CheckableArrayAccess arrayAccess -where - arrayAccess.canThrowOutOfBoundsDueToEmptyArray(sizeExpr, arrayCreation) and - sizeExpr = sink.getNode().asExpr() and - ImproperValidationOfArrayConstructionLocalFlow::flowPath(source, sink) -select arrayAccess.getIndexExpr(), source, sink, - "This accesses the $@, but the array is initialized using a $@ which may be zero.", arrayCreation, - "array", source.getNode(), "user-provided value" diff --git a/java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayIndexLocal.qhelp b/java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayIndexLocal.qhelp deleted file mode 100644 index 554a27b1fdc..00000000000 --- a/java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayIndexLocal.qhelp +++ /dev/null @@ -1,5 +0,0 @@ - - - diff --git a/java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayIndexLocal.ql b/java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayIndexLocal.ql deleted file mode 100644 index 7302ea676d1..00000000000 --- a/java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayIndexLocal.ql +++ /dev/null @@ -1,27 +0,0 @@ -/** - * @name Improper validation of local user-provided array index - * @description Using local user input as an index to an array, without - * proper validation, can lead to index out of bound exceptions. - * @kind path-problem - * @problem.severity recommendation - * @security-severity 8.8 - * @precision medium - * @id java/improper-validation-of-array-index-local - * @tags security - * external/cwe/cwe-129 - */ - -import java -import semmle.code.java.security.internal.ArraySizing -import semmle.code.java.security.ImproperValidationOfArrayIndexLocalQuery -import ImproperValidationOfArrayIndexLocalFlow::PathGraph - -from - ImproperValidationOfArrayIndexLocalFlow::PathNode source, - ImproperValidationOfArrayIndexLocalFlow::PathNode sink, CheckableArrayAccess arrayAccess -where - arrayAccess.canThrowOutOfBounds(sink.getNode().asExpr()) and - ImproperValidationOfArrayIndexLocalFlow::flowPath(source, sink) -select arrayAccess.getIndexExpr(), source, sink, - "This index depends on a $@ which can cause an ArrayIndexOutOfBoundsException.", source.getNode(), - "user-provided value" diff --git a/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayConstructionLocal.expected b/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayConstruction.expected similarity index 100% rename from java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayConstructionLocal.expected rename to java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayConstruction.expected diff --git a/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayConstruction.ext.yml b/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayConstruction.ext.yml new file mode 100644 index 00000000000..63507f47738 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayConstruction.ext.yml @@ -0,0 +1,6 @@ +extensions: + - addsTo: + pack: codeql/threat-models + extensible: threatModelConfiguration + data: + - ["local", true, 0] diff --git a/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayConstruction.qlref b/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayConstruction.qlref new file mode 100644 index 00000000000..02a179783ca --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayConstruction.qlref @@ -0,0 +1 @@ +Security/CWE/CWE-129/ImproperValidationOfArrayConstruction.ql diff --git a/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayConstructionLocal.qlref b/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayConstructionLocal.qlref deleted file mode 100644 index eee8ded0f3c..00000000000 --- a/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayConstructionLocal.qlref +++ /dev/null @@ -1 +0,0 @@ -Security/CWE/CWE-129/ImproperValidationOfArrayConstructionLocal.ql diff --git a/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayIndexLocal.expected b/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayIndex.expected similarity index 100% rename from java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayIndexLocal.expected rename to java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayIndex.expected diff --git a/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayIndex.ext.yml b/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayIndex.ext.yml new file mode 100644 index 00000000000..63507f47738 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayIndex.ext.yml @@ -0,0 +1,6 @@ +extensions: + - addsTo: + pack: codeql/threat-models + extensible: threatModelConfiguration + data: + - ["local", true, 0] diff --git a/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayIndex.qlref b/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayIndex.qlref new file mode 100644 index 00000000000..cb216ecd6a5 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayIndex.qlref @@ -0,0 +1 @@ +Security/CWE/CWE-129/ImproperValidationOfArrayIndex.ql diff --git a/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayIndexLocal.qlref b/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayIndexLocal.qlref deleted file mode 100644 index 12530489744..00000000000 --- a/java/ql/test/query-tests/security/CWE-129/semmle/tests/ImproperValidationOfArrayIndexLocal.qlref +++ /dev/null @@ -1 +0,0 @@ -Security/CWE/CWE-129/ImproperValidationOfArrayIndexLocal.ql From d05c5e3d94f2cc8f6f413b1f0cb34533099285e3 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Tue, 30 Apr 2024 15:50:36 +0200 Subject: [PATCH 07/14] Java: Deprecate the content of NumericCastTaintedLocalQuery, remove the local query variant and update the non-local query variant. --- .../java/security/NumericCastTaintedQuery.qll | 6 ++-- .../CWE/CWE-681/NumericCastTaintedLocal.qhelp | 5 ---- .../CWE/CWE-681/NumericCastTaintedLocal.ql | 29 ------------------- ...l.expected => NumericCastTainted.expected} | 0 .../semmle/tests/NumericCastTainted.ext.yml | 6 ++++ .../semmle/tests/NumericCastTainted.qlref | 1 + .../tests/NumericCastTaintedLocal.qlref | 1 - 7 files changed, 11 insertions(+), 37 deletions(-) delete mode 100644 java/ql/src/Security/CWE/CWE-681/NumericCastTaintedLocal.qhelp delete mode 100644 java/ql/src/Security/CWE/CWE-681/NumericCastTaintedLocal.ql rename java/ql/test/query-tests/security/CWE-681/semmle/tests/{NumericCastTaintedLocal.expected => NumericCastTainted.expected} (100%) create mode 100644 java/ql/test/query-tests/security/CWE-681/semmle/tests/NumericCastTainted.ext.yml create mode 100644 java/ql/test/query-tests/security/CWE-681/semmle/tests/NumericCastTainted.qlref delete mode 100644 java/ql/test/query-tests/security/CWE-681/semmle/tests/NumericCastTaintedLocal.qlref diff --git a/java/ql/lib/semmle/code/java/security/NumericCastTaintedQuery.qll b/java/ql/lib/semmle/code/java/security/NumericCastTaintedQuery.qll index b2754057b4c..b6bd505c38b 100644 --- a/java/ql/lib/semmle/code/java/security/NumericCastTaintedQuery.qll +++ b/java/ql/lib/semmle/code/java/security/NumericCastTaintedQuery.qll @@ -113,7 +113,7 @@ module NumericCastFlow = TaintTracking::Global; * A taint-tracking configuration for reasoning about local user input that is * used in a numeric cast. */ -module NumericCastLocalFlowConfig implements DataFlow::ConfigSig { +deprecated module NumericCastLocalFlowConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput } predicate isSink(DataFlow::Node sink) { @@ -134,6 +134,8 @@ module NumericCastLocalFlowConfig implements DataFlow::ConfigSig { } /** + * DEPRECATED: Use `NumericCastFlow` instead and configure threat model sources to include `local`. + * * Taint-tracking flow for local user input that is used in a numeric cast. */ -module NumericCastLocalFlow = TaintTracking::Global; +deprecated module NumericCastLocalFlow = TaintTracking::Global; diff --git a/java/ql/src/Security/CWE/CWE-681/NumericCastTaintedLocal.qhelp b/java/ql/src/Security/CWE/CWE-681/NumericCastTaintedLocal.qhelp deleted file mode 100644 index d225599aca1..00000000000 --- a/java/ql/src/Security/CWE/CWE-681/NumericCastTaintedLocal.qhelp +++ /dev/null @@ -1,5 +0,0 @@ - - - diff --git a/java/ql/src/Security/CWE/CWE-681/NumericCastTaintedLocal.ql b/java/ql/src/Security/CWE/CWE-681/NumericCastTaintedLocal.ql deleted file mode 100644 index 86bd1a5b048..00000000000 --- a/java/ql/src/Security/CWE/CWE-681/NumericCastTaintedLocal.ql +++ /dev/null @@ -1,29 +0,0 @@ -/** - * @name Local-user-controlled data in numeric cast - * @description Casting user-controlled numeric data to a narrower type without validation - * can cause unexpected truncation. - * @kind path-problem - * @problem.severity recommendation - * @security-severity 9.0 - * @precision medium - * @id java/tainted-numeric-cast-local - * @tags security - * external/cwe/cwe-197 - * external/cwe/cwe-681 - */ - -import java -import semmle.code.java.security.NumericCastTaintedQuery -import NumericCastLocalFlow::PathGraph - -from - NumericCastLocalFlow::PathNode source, NumericCastLocalFlow::PathNode sink, - NumericNarrowingCastExpr exp, VarAccess tainted -where - exp.getExpr() = tainted and - sink.getNode().asExpr() = tainted and - NumericCastLocalFlow::flowPath(source, sink) and - not exists(RightShiftOp e | e.getShiftedVariable() = tainted.getVariable()) -select exp, source, sink, - "This cast to a narrower type depends on a $@, potentially causing truncation.", source.getNode(), - "user-provided value" diff --git a/java/ql/test/query-tests/security/CWE-681/semmle/tests/NumericCastTaintedLocal.expected b/java/ql/test/query-tests/security/CWE-681/semmle/tests/NumericCastTainted.expected similarity index 100% rename from java/ql/test/query-tests/security/CWE-681/semmle/tests/NumericCastTaintedLocal.expected rename to java/ql/test/query-tests/security/CWE-681/semmle/tests/NumericCastTainted.expected diff --git a/java/ql/test/query-tests/security/CWE-681/semmle/tests/NumericCastTainted.ext.yml b/java/ql/test/query-tests/security/CWE-681/semmle/tests/NumericCastTainted.ext.yml new file mode 100644 index 00000000000..63507f47738 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-681/semmle/tests/NumericCastTainted.ext.yml @@ -0,0 +1,6 @@ +extensions: + - addsTo: + pack: codeql/threat-models + extensible: threatModelConfiguration + data: + - ["local", true, 0] diff --git a/java/ql/test/query-tests/security/CWE-681/semmle/tests/NumericCastTainted.qlref b/java/ql/test/query-tests/security/CWE-681/semmle/tests/NumericCastTainted.qlref new file mode 100644 index 00000000000..4f32338184f --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-681/semmle/tests/NumericCastTainted.qlref @@ -0,0 +1 @@ +Security/CWE/CWE-681/NumericCastTainted.ql diff --git a/java/ql/test/query-tests/security/CWE-681/semmle/tests/NumericCastTaintedLocal.qlref b/java/ql/test/query-tests/security/CWE-681/semmle/tests/NumericCastTaintedLocal.qlref deleted file mode 100644 index 708b9f635ee..00000000000 --- a/java/ql/test/query-tests/security/CWE-681/semmle/tests/NumericCastTaintedLocal.qlref +++ /dev/null @@ -1 +0,0 @@ -Security/CWE/CWE-681/NumericCastTaintedLocal.ql From b68abab12a6d66a58596ec45ad611e638cff59a4 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 1 May 2024 10:14:23 +0200 Subject: [PATCH 08/14] Java: Deprecate the content of ResponseSplittingLocalQuery and remove local query variant. --- .../security/ResponseSplittingLocalQuery.qll | 6 +++-- .../CWE/CWE-113/ResponseSplittingLocal.qhelp | 5 ----- .../CWE/CWE-113/ResponseSplittingLocal.ql | 22 ------------------- 3 files changed, 4 insertions(+), 29 deletions(-) delete mode 100644 java/ql/src/Security/CWE/CWE-113/ResponseSplittingLocal.qhelp delete mode 100644 java/ql/src/Security/CWE/CWE-113/ResponseSplittingLocal.ql diff --git a/java/ql/lib/semmle/code/java/security/ResponseSplittingLocalQuery.qll b/java/ql/lib/semmle/code/java/security/ResponseSplittingLocalQuery.qll index 23816caa1f8..e5845b630ec 100644 --- a/java/ql/lib/semmle/code/java/security/ResponseSplittingLocalQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ResponseSplittingLocalQuery.qll @@ -7,7 +7,7 @@ private import semmle.code.java.security.ResponseSplitting /** * A taint-tracking configuration to reason about response splitting vulnerabilities from local user input. */ -module ResponseSplittingLocalConfig implements DataFlow::ConfigSig { +deprecated module ResponseSplittingLocalConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput } predicate isSink(DataFlow::Node sink) { sink instanceof HeaderSplittingSink } @@ -32,6 +32,8 @@ module ResponseSplittingLocalConfig implements DataFlow::ConfigSig { } /** + * DEPRECATED: Use `ResponseSplittingFlow` instead and configure threat model sources to include `local`. + * * Taint-tracking flow for response splitting vulnerabilities from local user input. */ -module ResponseSplittingLocalFlow = TaintTracking::Global; +deprecated module ResponseSplittingLocalFlow = TaintTracking::Global; diff --git a/java/ql/src/Security/CWE/CWE-113/ResponseSplittingLocal.qhelp b/java/ql/src/Security/CWE/CWE-113/ResponseSplittingLocal.qhelp deleted file mode 100644 index 17afa6275fc..00000000000 --- a/java/ql/src/Security/CWE/CWE-113/ResponseSplittingLocal.qhelp +++ /dev/null @@ -1,5 +0,0 @@ - - - diff --git a/java/ql/src/Security/CWE/CWE-113/ResponseSplittingLocal.ql b/java/ql/src/Security/CWE/CWE-113/ResponseSplittingLocal.ql deleted file mode 100644 index 804ead11a35..00000000000 --- a/java/ql/src/Security/CWE/CWE-113/ResponseSplittingLocal.ql +++ /dev/null @@ -1,22 +0,0 @@ -/** - * @name HTTP response splitting from local source - * @description Writing user input directly to an HTTP header - * makes code vulnerable to attack by header splitting. - * @kind path-problem - * @problem.severity recommendation - * @security-severity 6.1 - * @precision medium - * @id java/http-response-splitting-local - * @tags security - * external/cwe/cwe-113 - */ - -import java -import semmle.code.java.security.ResponseSplittingLocalQuery -import ResponseSplittingLocalFlow::PathGraph - -from ResponseSplittingLocalFlow::PathNode source, ResponseSplittingLocalFlow::PathNode sink -where ResponseSplittingLocalFlow::flowPath(source, sink) -select sink.getNode(), source, sink, - "This header depends on a $@, which may cause a response-splitting vulnerability.", - source.getNode(), "user-provided value" From 5b89bd23c7afb05aaaab2ae12dcb065fdff16cf0 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 1 May 2024 10:33:46 +0200 Subject: [PATCH 09/14] Java: Deprecate the content of SqlTaintedLocalQuery and remove the local query variant. --- .../java/security/SqlTaintedLocalQuery.qll | 6 +++-- .../CWE/CWE-089/SqlTaintedLocal.qhelp | 5 ---- .../Security/CWE/CWE-089/SqlTaintedLocal.ql | 24 ------------------- ...ntedLocal.expected => SqlTainted.expected} | 0 .../semmle/examples/SqlTainted.ext.yml | 6 +++++ .../CWE-089/semmle/examples/SqlTainted.qlref | 1 + .../semmle/examples/SqlTaintedLocal.qlref | 1 - 7 files changed, 11 insertions(+), 32 deletions(-) delete mode 100644 java/ql/src/Security/CWE/CWE-089/SqlTaintedLocal.qhelp delete mode 100644 java/ql/src/Security/CWE/CWE-089/SqlTaintedLocal.ql rename java/ql/test/query-tests/security/CWE-089/semmle/examples/{SqlTaintedLocal.expected => SqlTainted.expected} (100%) create mode 100644 java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlTainted.ext.yml create mode 100644 java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlTainted.qlref delete mode 100644 java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlTaintedLocal.qlref diff --git a/java/ql/lib/semmle/code/java/security/SqlTaintedLocalQuery.qll b/java/ql/lib/semmle/code/java/security/SqlTaintedLocalQuery.qll index 9f32bd00b57..7ff4b300ce8 100644 --- a/java/ql/lib/semmle/code/java/security/SqlTaintedLocalQuery.qll +++ b/java/ql/lib/semmle/code/java/security/SqlTaintedLocalQuery.qll @@ -12,7 +12,7 @@ private import semmle.code.java.security.Sanitizers * A taint-tracking configuration for reasoning about local user input that is * used in a SQL query. */ -module LocalUserInputToQueryInjectionFlowConfig implements DataFlow::ConfigSig { +deprecated module LocalUserInputToQueryInjectionFlowConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput } predicate isSink(DataFlow::Node sink) { sink instanceof QueryInjectionSink } @@ -25,7 +25,9 @@ module LocalUserInputToQueryInjectionFlowConfig implements DataFlow::ConfigSig { } /** + * DEPRECATED: Use `QueryInjectionFlow` instead and configure threat model sources to include `local`. + * * Taint-tracking flow for local user input that is used in a SQL query. */ -module LocalUserInputToQueryInjectionFlow = +deprecated module LocalUserInputToQueryInjectionFlow = TaintTracking::Global; diff --git a/java/ql/src/Security/CWE/CWE-089/SqlTaintedLocal.qhelp b/java/ql/src/Security/CWE/CWE-089/SqlTaintedLocal.qhelp deleted file mode 100644 index accf2aee854..00000000000 --- a/java/ql/src/Security/CWE/CWE-089/SqlTaintedLocal.qhelp +++ /dev/null @@ -1,5 +0,0 @@ - - - diff --git a/java/ql/src/Security/CWE/CWE-089/SqlTaintedLocal.ql b/java/ql/src/Security/CWE/CWE-089/SqlTaintedLocal.ql deleted file mode 100644 index 8b95ee597be..00000000000 --- a/java/ql/src/Security/CWE/CWE-089/SqlTaintedLocal.ql +++ /dev/null @@ -1,24 +0,0 @@ -/** - * @name Query built from local-user-controlled sources - * @description Building a SQL or Java Persistence query from user-controlled sources is vulnerable to insertion of - * malicious code by the user. - * @kind path-problem - * @problem.severity recommendation - * @security-severity 8.8 - * @precision medium - * @id java/sql-injection-local - * @tags security - * external/cwe/cwe-089 - * external/cwe/cwe-564 - */ - -import java -import semmle.code.java.security.SqlTaintedLocalQuery -import LocalUserInputToQueryInjectionFlow::PathGraph - -from - LocalUserInputToQueryInjectionFlow::PathNode source, - LocalUserInputToQueryInjectionFlow::PathNode sink -where LocalUserInputToQueryInjectionFlow::flowPath(source, sink) -select sink.getNode(), source, sink, "This query depends on a $@.", source.getNode(), - "user-provided value" diff --git a/java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlTaintedLocal.expected b/java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlTainted.expected similarity index 100% rename from java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlTaintedLocal.expected rename to java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlTainted.expected diff --git a/java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlTainted.ext.yml b/java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlTainted.ext.yml new file mode 100644 index 00000000000..63507f47738 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlTainted.ext.yml @@ -0,0 +1,6 @@ +extensions: + - addsTo: + pack: codeql/threat-models + extensible: threatModelConfiguration + data: + - ["local", true, 0] diff --git a/java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlTainted.qlref b/java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlTainted.qlref new file mode 100644 index 00000000000..21a12e5eadd --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlTainted.qlref @@ -0,0 +1 @@ +Security/CWE/CWE-089/SqlTainted.ql diff --git a/java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlTaintedLocal.qlref b/java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlTaintedLocal.qlref deleted file mode 100644 index ac5a020be5a..00000000000 --- a/java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlTaintedLocal.qlref +++ /dev/null @@ -1 +0,0 @@ -Security/CWE/CWE-089/SqlTaintedLocal.ql From ed7538d0b982c60538a30912911da45210be253e Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 1 May 2024 10:39:49 +0200 Subject: [PATCH 10/14] Java: Deprecate the local content of TaintedPathQuery and remove the local query variant. --- .../code/java/security/TaintedPathQuery.qll | 10 +++++--- .../CWE/CWE-022/TaintedPathLocal.qhelp | 5 ---- .../Security/CWE/CWE-022/TaintedPathLocal.ql | 24 ------------------- 3 files changed, 7 insertions(+), 32 deletions(-) delete mode 100644 java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.qhelp delete mode 100644 java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.ql diff --git a/java/ql/lib/semmle/code/java/security/TaintedPathQuery.qll b/java/ql/lib/semmle/code/java/security/TaintedPathQuery.qll index 63bd4949699..c396b48a7b8 100644 --- a/java/ql/lib/semmle/code/java/security/TaintedPathQuery.qll +++ b/java/ql/lib/semmle/code/java/security/TaintedPathQuery.qll @@ -80,7 +80,7 @@ module TaintedPathFlow = TaintTracking::Global; /** * A taint-tracking configuration for tracking flow from local user input to the creation of a path. */ -module TaintedPathLocalConfig implements DataFlow::ConfigSig { +deprecated module TaintedPathLocalConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput } predicate isSink(DataFlow::Node sink) { sink instanceof TaintedPathSink } @@ -95,5 +95,9 @@ module TaintedPathLocalConfig implements DataFlow::ConfigSig { } } -/** Tracks flow from local user input to the creation of a path. */ -module TaintedPathLocalFlow = TaintTracking::Global; +/** + * DEPRECATED: Use `TaintedPathFlow` instead and configure threat model sources to include `local`. + * + * Tracks flow from local user input to the creation of a path. + */ +deprecated module TaintedPathLocalFlow = TaintTracking::Global; diff --git a/java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.qhelp b/java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.qhelp deleted file mode 100644 index 25fb4f6153a..00000000000 --- a/java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.qhelp +++ /dev/null @@ -1,5 +0,0 @@ - - - diff --git a/java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.ql b/java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.ql deleted file mode 100644 index 60dc6b54be8..00000000000 --- a/java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.ql +++ /dev/null @@ -1,24 +0,0 @@ -/** - * @name Local-user-controlled data in path expression - * @description Accessing paths influenced by users can allow an attacker to access unexpected resources. - * @kind path-problem - * @problem.severity recommendation - * @security-severity 7.5 - * @precision medium - * @id java/path-injection-local - * @tags security - * external/cwe/cwe-022 - * external/cwe/cwe-023 - * external/cwe/cwe-036 - * external/cwe/cwe-073 - */ - -import java -import semmle.code.java.security.PathCreation -import semmle.code.java.security.TaintedPathQuery -import TaintedPathLocalFlow::PathGraph - -from TaintedPathLocalFlow::PathNode source, TaintedPathLocalFlow::PathNode sink -where TaintedPathLocalFlow::flowPath(source, sink) -select sink.getNode(), source, sink, "This path depends on a $@.", source.getNode(), - "user-provided value" From d9c7401ea2e4dc008ea014de7828a29237252099 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 1 May 2024 10:56:19 +0200 Subject: [PATCH 11/14] Java: Deprecate the local content of UrlRedirectLocalQuery and remove the local query variant. --- .../java/security/UrlRedirectLocalQuery.qll | 6 ++++-- .../CWE/CWE-601/UrlRedirectLocal.qhelp | 5 ----- .../Security/CWE/CWE-601/UrlRedirectLocal.ql | 21 ------------------- 3 files changed, 4 insertions(+), 28 deletions(-) delete mode 100644 java/ql/src/Security/CWE/CWE-601/UrlRedirectLocal.qhelp delete mode 100644 java/ql/src/Security/CWE/CWE-601/UrlRedirectLocal.ql diff --git a/java/ql/lib/semmle/code/java/security/UrlRedirectLocalQuery.qll b/java/ql/lib/semmle/code/java/security/UrlRedirectLocalQuery.qll index 8b2e0374322..f68fb959ea5 100644 --- a/java/ql/lib/semmle/code/java/security/UrlRedirectLocalQuery.qll +++ b/java/ql/lib/semmle/code/java/security/UrlRedirectLocalQuery.qll @@ -7,13 +7,15 @@ private import semmle.code.java.security.UrlRedirect /** * A taint-tracking configuration to reason about URL redirection from local sources. */ -module UrlRedirectLocalConfig implements DataFlow::ConfigSig { +deprecated module UrlRedirectLocalConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput } predicate isSink(DataFlow::Node sink) { sink instanceof UrlRedirectSink } } /** + * DEPRECATED: Use `UrlRedirectFlow` instead and configure threat model sources to include `local`. + * * Taint-tracking flow for URL redirection from local sources. */ -module UrlRedirectLocalFlow = TaintTracking::Global; +deprecated module UrlRedirectLocalFlow = TaintTracking::Global; diff --git a/java/ql/src/Security/CWE/CWE-601/UrlRedirectLocal.qhelp b/java/ql/src/Security/CWE/CWE-601/UrlRedirectLocal.qhelp deleted file mode 100644 index 05e5cf6fb49..00000000000 --- a/java/ql/src/Security/CWE/CWE-601/UrlRedirectLocal.qhelp +++ /dev/null @@ -1,5 +0,0 @@ - - - diff --git a/java/ql/src/Security/CWE/CWE-601/UrlRedirectLocal.ql b/java/ql/src/Security/CWE/CWE-601/UrlRedirectLocal.ql deleted file mode 100644 index 0ba8f5ec38c..00000000000 --- a/java/ql/src/Security/CWE/CWE-601/UrlRedirectLocal.ql +++ /dev/null @@ -1,21 +0,0 @@ -/** - * @name URL redirection from local source - * @description URL redirection based on unvalidated user-input - * may cause redirection to malicious web sites. - * @kind path-problem - * @problem.severity recommendation - * @security-severity 6.1 - * @precision medium - * @id java/unvalidated-url-redirection-local - * @tags security - * external/cwe/cwe-601 - */ - -import java -import semmle.code.java.security.UrlRedirectLocalQuery -import UrlRedirectLocalFlow::PathGraph - -from UrlRedirectLocalFlow::PathNode source, UrlRedirectLocalFlow::PathNode sink -where UrlRedirectLocalFlow::flowPath(source, sink) -select sink.getNode(), source, sink, "Untrusted URL redirection depends on a $@.", source.getNode(), - "user-provided value" From 58bbfe694f165955a6d59ecec011d7b3bdebd6e1 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 1 May 2024 11:09:09 +0200 Subject: [PATCH 12/14] Java: Deprecate the content of ExecTaintedLocalQuery as this is unused. --- .../lib/semmle/code/java/security/ExecTaintedLocalQuery.qll | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/java/ql/lib/semmle/code/java/security/ExecTaintedLocalQuery.qll b/java/ql/lib/semmle/code/java/security/ExecTaintedLocalQuery.qll index ea36338fcb9..7a2d5b0947d 100644 --- a/java/ql/lib/semmle/code/java/security/ExecTaintedLocalQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ExecTaintedLocalQuery.qll @@ -7,7 +7,7 @@ private import semmle.code.java.security.CommandArguments private import semmle.code.java.security.Sanitizers /** A taint-tracking configuration to reason about use of externally controlled strings to make command line commands. */ -module ExecTaintedLocalConfig implements DataFlow::ConfigSig { +deprecated module ExecTaintedLocalConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput } predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof ArgumentToExec } @@ -20,6 +20,8 @@ module ExecTaintedLocalConfig implements DataFlow::ConfigSig { } /** + * DEPRCATED: Unused. + * * Taint-tracking flow for use of externally controlled strings to make command line commands. */ -module ExecTaintedLocalFlow = TaintTracking::Global; +deprecated module ExecTaintedLocalFlow = TaintTracking::Global; From 42653b5fecb9ab06b90e63fb9d0a0c7e0a01e9fc Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 1 May 2024 11:39:58 +0200 Subject: [PATCH 13/14] Java: Add change note about local query removal. --- .../change-notes/2024-05-01-remove-local-query-variants.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 java/ql/src/change-notes/2024-05-01-remove-local-query-variants.md diff --git a/java/ql/src/change-notes/2024-05-01-remove-local-query-variants.md b/java/ql/src/change-notes/2024-05-01-remove-local-query-variants.md new file mode 100644 index 00000000000..9f137fc04eb --- /dev/null +++ b/java/ql/src/change-notes/2024-05-01-remove-local-query-variants.md @@ -0,0 +1,4 @@ +--- +category: majorAnalysis +--- +* Removed `local` query variants. The results pertaining to local sources can be found using the non-local counterpart query. As an example, the results previously found by `java/unvalidated-url-redirection-local` can be found by `java/unvalidated-url-redirection`, if the `local` threat model is added. The removed queries are `java/path-injection-local`, `java/command-line-injection-local`, `java/xss-local`, `java/sql-injection-local`, `java/http-response-splitting-local`, `java/improper-validation-of-array-construction-local`, `java/improper-validation-of-array-index-local`, `java/tainted-format-string-local`, `java/tainted-arithmetic-local`, `java/unvalidated-url-redirection-local`, `java/xxe-local` and `java/tainted-numeric-cast-local`. From 8b0f3af5b1b7b08cda1b8de7ee449a7ba672c326 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Thu, 2 May 2024 14:05:44 +0200 Subject: [PATCH 14/14] Java: Update change-note. --- .../change-notes/2024-05-01-remove-local-query-variants.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/java/ql/src/change-notes/2024-05-01-remove-local-query-variants.md b/java/ql/src/change-notes/2024-05-01-remove-local-query-variants.md index 9f137fc04eb..dbf638969ff 100644 --- a/java/ql/src/change-notes/2024-05-01-remove-local-query-variants.md +++ b/java/ql/src/change-notes/2024-05-01-remove-local-query-variants.md @@ -1,4 +1,4 @@ --- -category: majorAnalysis +category: breaking --- -* Removed `local` query variants. The results pertaining to local sources can be found using the non-local counterpart query. As an example, the results previously found by `java/unvalidated-url-redirection-local` can be found by `java/unvalidated-url-redirection`, if the `local` threat model is added. The removed queries are `java/path-injection-local`, `java/command-line-injection-local`, `java/xss-local`, `java/sql-injection-local`, `java/http-response-splitting-local`, `java/improper-validation-of-array-construction-local`, `java/improper-validation-of-array-index-local`, `java/tainted-format-string-local`, `java/tainted-arithmetic-local`, `java/unvalidated-url-redirection-local`, `java/xxe-local` and `java/tainted-numeric-cast-local`. +* Removed `local` query variants. The results pertaining to local sources can be found using the non-local counterpart query. As an example, the results previously found by `java/unvalidated-url-redirection-local` can be found by `java/unvalidated-url-redirection`, if the `local` threat model is enabled. The removed queries are `java/path-injection-local`, `java/command-line-injection-local`, `java/xss-local`, `java/sql-injection-local`, `java/http-response-splitting-local`, `java/improper-validation-of-array-construction-local`, `java/improper-validation-of-array-index-local`, `java/tainted-format-string-local`, `java/tainted-arithmetic-local`, `java/unvalidated-url-redirection-local`, `java/xxe-local` and `java/tainted-numeric-cast-local`.