From dfd85c321ca8c99f1986f4ab0179304a85232a12 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Tue, 28 Apr 2026 22:02:32 +0100 Subject: [PATCH 1/2] C++: Compute 'IgnorableOperationToOperationSourceCandidateConfig' after an initial round of the query to reduce the number of sinks. --- .../UncheckedLeapYearAfterYearModification.ql | 144 +++++++++++------- 1 file changed, 87 insertions(+), 57 deletions(-) diff --git a/cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql b/cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql index 0a52a2b0ff4c..1320069bb1ca 100644 --- a/cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql +++ b/cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql @@ -227,6 +227,30 @@ class IgnorableUnaryBitwiseOperation extends IgnorableOperation instanceof Unary class IgnorableAssignmentBitwiseOperation extends IgnorableOperation instanceof AssignBitwiseOperation { } +class YearFieldAssignmentNode extends DataFlow::Node { + YearFieldAccess access; + + YearFieldAssignmentNode() { + exists(Function f | + f = this.getEnclosingCallable().getUnderlyingCallable() and not f instanceof IgnorableFunction + | + this.asDefinition().(Assignment).getLValue() = access + or + this.asDefinition().(CrementOperation).getOperand() = access + or + exists(Call c | c.getAnArgument() = access and this.asDefiningArgument() = access) + or + exists(Call c, AddressOfExpr aoe | + c.getAnArgument() = aoe and + aoe.getOperand() = access and + this.asDefiningArgument() = aoe + ) + ) + } + + YearFieldAccess getYearFieldAccess() { result = access } +} + /** * An arithmetic operation where one of the operands is a pointer or char type, ignore it */ @@ -287,24 +311,7 @@ predicate isOperationSourceCandidate(Expr e) { } /** - * A data flow that tracks an ignorable operation (such as a bitwise operation) to an operation source, so we may disqualify it. - */ -module IgnorableOperationToOperationSourceCandidateConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node n) { n.asExpr() instanceof IgnorableOperation } - - predicate isSink(DataFlow::Node n) { isOperationSourceCandidate(n.asExpr()) } - - // looking for sources and sinks in the same function - DataFlow::FlowFeature getAFeature() { - result instanceof DataFlow::FeatureEqualSourceSinkCallContext - } -} - -module IgnorableOperationToOperationSourceCandidateFlow = - TaintTracking::Global; - -/** - * The set of all expressions which is a candidate expression and also does not flow from to to some ignorable expression (eg. bitwise op) + * The set of all expressions which is a candidate expression. * ``` * a = something <<< 2; * myDate.year = a + 1; // invalid @@ -314,49 +321,16 @@ module IgnorableOperationToOperationSourceCandidateFlow = * ``` */ class OperationSource extends Expr { - OperationSource() { - isOperationSourceCandidate(this) and - // If the candidate came from an ignorable operation, ignore the candidate - // NOTE: we cannot easily flow the candidate to an ignorable operation as that can - // be tricky in practice, e.g., a mod operation on a year would be part of a leap year check - // but a mod operation ending in a year is more indicative of something to ignore (a conversion) - not exists(IgnorableOperationToOperationSourceCandidateFlow::PathNode sink | - sink.getNode().asExpr() = this and - sink.isSink() - ) - } -} - -class YearFieldAssignmentNode extends DataFlow::Node { - YearFieldAccess access; - - YearFieldAssignmentNode() { - exists(Function f | - f = this.getEnclosingCallable().getUnderlyingCallable() and not f instanceof IgnorableFunction - ) and - ( - this.asDefinition().(Assignment).getLValue() = access - or - this.asDefinition().(CrementOperation).getOperand() = access - or - exists(Call c | c.getAnArgument() = access and this.asDefiningArgument() = access) - or - exists(Call c, AddressOfExpr aoe | - c.getAnArgument() = aoe and - aoe.getOperand() = access and - this.asDefiningArgument() = aoe - ) - ) - } - - YearFieldAccess getYearFieldAccess() { result = access } + OperationSource() { isOperationSourceCandidate(this) } } /** - * A DataFlow configuration for identifying flows from an identified source - * to the Year field of a date object. + * An initial DataFlow configuration for identifying flows from an identified source + * to the Year field of a date object. This is used to restrict the sinks of + * `IgnorableOperationToOperationSourceCandidateConfig` and the sinks of the + * final `OperationToYearAssignmentConfig`. */ -module OperationToYearAssignmentConfig implements DataFlow::ConfigSig { +module OperationToYearAssignmentConfig0 implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node n) { n.asExpr() instanceof OperationSource } predicate isSink(DataFlow::Node n) { @@ -411,6 +385,62 @@ module OperationToYearAssignmentConfig implements DataFlow::ConfigSig { predicate isBarrierOut(DataFlow::Node n) { isSink(n) } } +module OperationToYearAssignmentFlow0 = TaintTracking::Global; + +predicate yearAssignmentFlowsFromSource(DataFlow::Node source, DataFlow::Node sink) { + OperationToYearAssignmentFlow0::flow(source, sink) +} + +/** + * A data flow that tracks an ignorable operation (such as a bitwise operation) to an operation source, so we may disqualify it. + * Sinks are restricted to operation source candidates that have a flow to a year assignment in `OperationToYearAssignmentFlow0`. + */ +module IgnorableOperationToOperationSourceCandidateConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node n) { n.asExpr() instanceof IgnorableOperation } + + predicate isSink(DataFlow::Node n) { + isOperationSourceCandidate(n.asExpr()) and + yearAssignmentFlowsFromSource(n, _) + } + + // looking for sources and sinks in the same function + DataFlow::FlowFeature getAFeature() { + result instanceof DataFlow::FeatureEqualSourceSinkCallContext + } +} + +module IgnorableOperationToOperationSourceCandidateFlow = + TaintTracking::Global; + +/** + * The final DataFlow configuration that refines `OperationToYearAssignmentConfig0` by + * additionally filtering out operation sources that flow from an ignorable operation + * (via `IgnorableOperationToOperationSourceCandidateFlow`). + */ +module OperationToYearAssignmentConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node n) { yearAssignmentFlowsFromSource(n, _) } + + predicate isSink(DataFlow::Node n) { + exists(DataFlow::Node operation | + yearAssignmentFlowsFromSource(operation, n) and + // If the candidate came from an ignorable operation, ignore the candidate + // NOTE: we cannot easily flow the candidate to an ignorable operation as that can + // be tricky in practice, e.g., a mod operation on a year would be part of a leap year check + // but a mod operation ending in a year is more indicative of something to ignore (a conversion) + not exists(IgnorableOperationToOperationSourceCandidateFlow::PathNode sink | + sink.getNode() = operation and + sink.isSink() + ) + ) + } + + predicate isBarrier(DataFlow::Node n) { OperationToYearAssignmentConfig0::isBarrier(n) } + + predicate isBarrierIn(DataFlow::Node n) { isSource(n) } + + predicate isBarrierOut(DataFlow::Node n) { isSink(n) } +} + module OperationToYearAssignmentFlow = TaintTracking::Global; predicate isLeapYearCheckSink(DataFlow::Node sink) { From 96d6ee61ffde3ce28df1feec97b1e0200ed5bc13 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 29 Apr 2026 10:55:02 +0100 Subject: [PATCH 2/2] Update cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com> --- .../Leap Year/UncheckedLeapYearAfterYearModification.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql b/cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql index 1320069bb1ca..4bc58eb08540 100644 --- a/cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql +++ b/cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql @@ -311,7 +311,7 @@ predicate isOperationSourceCandidate(Expr e) { } /** - * The set of all expressions which is a candidate expression. + * The set of all expressions that are candidate expression. * ``` * a = something <<< 2; * myDate.year = a + 1; // invalid