From 072f19008affa508b36de74ab42ef4aaa9a05398 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Tue, 30 Apr 2024 13:17:19 +0200 Subject: [PATCH] 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