mirror of
https://github.com/github/codeql.git
synced 2025-12-16 16:53:25 +01:00
Made changes requested by reviewers - bounded() for range checking, style and better comments
This commit is contained in:
@@ -6,7 +6,7 @@ import semmle.code.java.dataflow.TaintTracking
|
||||
import semmle.code.java.security.SensitiveActions
|
||||
import semmle.code.java.frameworks.android.Compose
|
||||
private import semmle.code.java.security.Sanitizers
|
||||
import semmle.code.java.Constants
|
||||
private import semmle.code.java.dataflow.RangeAnalysis
|
||||
|
||||
/** A data flow source node for sensitive logging sources. */
|
||||
abstract class SensitiveLoggerSource extends DataFlow::Node { }
|
||||
@@ -49,12 +49,12 @@ private class TypeType extends RefType {
|
||||
*
|
||||
* It allows for substring operations taking the first N (or last N, for Kotlin) characters, limited to 7 or fewer.
|
||||
*/
|
||||
private class SensitiveLoggerSanitizerCalled extends SensitiveLoggerBarrier {
|
||||
SensitiveLoggerSanitizerCalled() {
|
||||
private class PrefixSuffixBarrier extends SensitiveLoggerBarrier {
|
||||
PrefixSuffixBarrier() {
|
||||
exists(MethodCall mc, Method m, int limit |
|
||||
limit = 7 and
|
||||
mc.getMethod() = m and
|
||||
(
|
||||
mc.getMethod() = m
|
||||
|
|
||||
// substring in Java
|
||||
(
|
||||
m.hasQualifiedName("java.lang", "String", "substring") or
|
||||
@@ -80,79 +80,34 @@ private class SensitiveLoggerSanitizerCalled extends SensitiveLoggerBarrier {
|
||||
) and
|
||||
this.asExpr() = mc.getArgument(0)
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/** A predicate to check single-argument method calls for a constant integer below a set limit. */
|
||||
bindingset[limit, isKotlin]
|
||||
private predicate singleArgLimit(MethodCall mc, int limit, boolean isKotlin) {
|
||||
exists(int argIndex, int staticInt |
|
||||
exists(int argIndex |
|
||||
(if isKotlin = true then argIndex = 1 else argIndex = 0) and
|
||||
(
|
||||
staticInt <= limit and
|
||||
staticInt > 0 and
|
||||
mc.getArgument(argIndex).getUnderlyingExpr().(CompileTimeConstantExpr).getIntValue() =
|
||||
staticInt
|
||||
or
|
||||
exists(CompileTimeConstantExpr cte, DataFlow::Node source, DataFlow::Node sink |
|
||||
source.asExpr() = cte and
|
||||
cte.getIntValue() = staticInt and
|
||||
sink.asExpr() = mc.getArgument(argIndex) and
|
||||
IntegerToArgFlow::flow(source, sink)
|
||||
)
|
||||
)
|
||||
bounded(mc.getArgument(argIndex), any(ZeroBound z), limit, true, _)
|
||||
)
|
||||
}
|
||||
|
||||
/** A predicate to check two-argument method calls for zero and a constant integer below a set limit. */
|
||||
bindingset[limit, isKotlin]
|
||||
private predicate twoArgLimit(MethodCall mc, int limit, boolean isKotlin) {
|
||||
exists(int firstArgIndex, int secondArgIndex, int staticInt |
|
||||
staticInt <= limit and
|
||||
staticInt > 0 and
|
||||
exists(int firstArgIndex, int secondArgIndex |
|
||||
(
|
||||
isKotlin = true and firstArgIndex = 1 and secondArgIndex = 2
|
||||
or
|
||||
isKotlin = false and firstArgIndex = 0 and secondArgIndex = 1
|
||||
) and
|
||||
mc.getArgument(firstArgIndex).getUnderlyingExpr().(CompileTimeConstantExpr).getIntValue() = 0 and
|
||||
(
|
||||
mc.getArgument(secondArgIndex).getUnderlyingExpr().(CompileTimeConstantExpr).getIntValue() =
|
||||
staticInt
|
||||
or
|
||||
exists(CompileTimeConstantExpr cte, DataFlow::Node source, DataFlow::Node sink |
|
||||
source.asExpr() = cte and
|
||||
cte.getIntValue() = staticInt and
|
||||
sink.asExpr() = mc.getArgument(secondArgIndex) and
|
||||
IntegerToArgFlow::flow(source, sink)
|
||||
)
|
||||
)
|
||||
bounded(mc.getArgument(secondArgIndex), any(ZeroBound z), limit, true, _)
|
||||
)
|
||||
}
|
||||
|
||||
/** A data-flow configuration for identifying flow from a constant integer to a use in a method argument. */
|
||||
private module IntegerToArgConfig implements DataFlow::ConfigSig {
|
||||
predicate isSource(DataFlow::Node source) {
|
||||
source.asExpr().getUnderlyingExpr() instanceof CompileTimeConstantExpr and
|
||||
source.asExpr().getType() instanceof IntegralType and
|
||||
source.asExpr().(CompileTimeConstantExpr).getIntValue() > 0
|
||||
}
|
||||
|
||||
predicate isSink(DataFlow::Node sink) {
|
||||
exists(MethodCall mc |
|
||||
sink.asExpr() = mc.getAnArgument() and
|
||||
sink.asExpr().getType() instanceof IntegralType
|
||||
)
|
||||
}
|
||||
|
||||
predicate isBarrier(DataFlow::Node sanitizer) { none() }
|
||||
|
||||
predicate isBarrierIn(DataFlow::Node node) { none() }
|
||||
}
|
||||
|
||||
private class GenericSanitizer extends SensitiveLoggerBarrier {
|
||||
GenericSanitizer() {
|
||||
private class DefaultSensitiveLoggerBarrier extends SensitiveLoggerBarrier {
|
||||
DefaultSensitiveLoggerBarrier() {
|
||||
this.asExpr() instanceof LiveLiteral or
|
||||
this instanceof SimpleTypeSanitizer or
|
||||
this.getType() instanceof TypeType
|
||||
@@ -173,5 +128,3 @@ module SensitiveLoggerConfig implements DataFlow::ConfigSig {
|
||||
}
|
||||
|
||||
module SensitiveLoggerFlow = TaintTracking::Global<SensitiveLoggerConfig>;
|
||||
|
||||
module IntegerToArgFlow = TaintTracking::Global<IntegerToArgConfig>;
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
---
|
||||
category: minorAnalysis
|
||||
---
|
||||
* Calls to `substring` (for Java), `take` (for Kotlin) and similar functions, when called with a fixed length less than or equal to 7, are now treated as sanitizers for the `java/sensitive-log` query.
|
||||
* Operations that extract only a fixed-length prefix or suffix of a string (for example, `substring` in Java or `take` in Kotlin), when limited to a length of at most 7 characters, are now treated as sanitizers for the `java/sensitive-log` query.
|
||||
@@ -11,5 +11,7 @@ class Test {
|
||||
logger.error("Auth failed for: " + stringTokenizer); // Safe
|
||||
logger.error("Auth failed for: " + authToken.substring(4) + "..."); // Safe
|
||||
logger.error("Auth failed for: " + authToken.substring(0,4) + "..."); // Safe
|
||||
logger.error("Auth failed for: " + authToken.substring(1,5) + "..."); // $ Alert
|
||||
logger.error("Auth failed for: " + authToken.substring(0,8) + "..."); // $ Alert
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user