Skip to content

Commit ec3b350

Browse files
committed
Misc. tweaks addressing FPs and cases observed during auditing.
1 parent 800abae commit ec3b350

1 file changed

Lines changed: 24 additions & 47 deletions

File tree

cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModificationPrecise.ql

Lines changed: 24 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ predicate isLikelyConversionConstant(int c) {
7272
c = 2400001 or
7373
c = 2400000 or
7474
c = 2141 or // fixed‑point month/day extraction
75+
c = 2000 or
7576
c = 65536 or
7677
c = 7834 or
7778
c = 256 or
@@ -95,6 +96,9 @@ class IgnorableConstantArithmetic extends IgnorableOperation {
9596
}
9697
}
9798

99+
// If a unary minus assume it is some sort of conversion
100+
class IgnorableUnaryMinus extends IgnorableOperation instanceof UnaryMinusExpr { }
101+
98102
class IgnorableBinaryBitwiseOperation extends IgnorableOperation instanceof BinaryBitwiseOperation {
99103
}
100104

@@ -275,44 +279,33 @@ module OperationToYearAssignmentConfig implements DataFlow::ConfigSig {
275279
n.asExpr().getUnspecifiedType() instanceof CharType
276280
or
277281
n.asExpr() instanceof IgnorableOperation
282+
or
283+
isLeapYearCheckSink(n)
278284
}
279285

286+
// Block flow out of an operation source to get the "closest" operation to the sink
287+
predicate isBarrierIn(DataFlow::Node n) { isSource(n) }
288+
280289
predicate isBarrierOut(DataFlow::Node n) { isSink(n) }
281290
// DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext }
282291
}
283292

284293
module OperationToYearAssignmentFlow = TaintTracking::Global<OperationToYearAssignmentConfig>;
285294

286-
/**
287-
* A Dataflow configuration for tracing from one OperationToYearAssignmentFlow source to another OperationToYearAssignmentFlow source.
288-
*/
289-
module KnownYearOpSourceToKnownYearOpSourceConfig implements DataFlow::ConfigSig {
290-
predicate isSource(DataFlow::Node n) {
291-
exists(OperationToYearAssignmentFlow::PathNode src |
292-
src.getNode().asExpr() = n.asExpr() and
293-
src.isSource()
294-
)
295-
}
296-
297-
predicate isSink(DataFlow::Node n) {
298-
exists(OperationToYearAssignmentFlow::PathNode src |
299-
src.getNode().asExpr() = n.asExpr() and
300-
src.isSource()
295+
predicate isLeapYearCheckSink(DataFlow::Node sink) {
296+
// Local leap year check
297+
sink.asExpr().(LeapYearFieldAccess).isUsedInCorrectLeapYearCheck()
298+
or
299+
// passed to function that seems to check for leap year
300+
exists(ChecksForLeapYearFunctionCall fc |
301+
sink.asExpr() = fc.getAnArgument()
302+
or
303+
sink.asExpr() = fc.getAnArgument().(FieldAccess).getQualifier()
304+
or
305+
exists(AddressOfExpr aoe |
306+
fc.getAnArgument() = aoe and
307+
aoe.getOperand() = sink.asExpr()
301308
)
302-
}
303-
}
304-
305-
module KnownYearOpSourceToKnownYearOpSourceFlow =
306-
TaintTracking::Global<KnownYearOpSourceToKnownYearOpSourceConfig>;
307-
308-
/**
309-
* There does not exist an OperationSource that flows through this given OperationSource expression.
310-
*/
311-
predicate isRootOperationSource(OperationSource e) {
312-
not exists(DataFlow::Node src, DataFlow::Node sink |
313-
src != sink and
314-
KnownYearOpSourceToKnownYearOpSourceFlow::flow(src, sink) and
315-
sink.asExpr() = e
316309
)
317310
}
318311

@@ -322,22 +315,7 @@ predicate isRootOperationSource(OperationSource e) {
322315
module YearFieldAccessToLeapYearCheckConfig implements DataFlow::ConfigSig {
323316
predicate isSource(DataFlow::Node source) { source instanceof YearFieldAssignmentNode }
324317

325-
predicate isSink(DataFlow::Node sink) {
326-
// Local leap year check
327-
sink.asExpr().(LeapYearFieldAccess).isUsedInCorrectLeapYearCheck()
328-
or
329-
// passed to function that seems to check for leap year
330-
exists(ChecksForLeapYearFunctionCall fc |
331-
sink.asExpr() = fc.getAnArgument()
332-
or
333-
sink.asExpr() = fc.getAnArgument().(FieldAccess).getQualifier()
334-
or
335-
exists(AddressOfExpr aoe |
336-
fc.getAnArgument() = aoe and
337-
aoe.getOperand() = sink.asExpr()
338-
)
339-
)
340-
}
318+
predicate isSink(DataFlow::Node sink) { isLeapYearCheckSink(sink) }
341319

342320
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
343321
// flow from a YearFieldAccess to the qualifier
@@ -363,7 +341,7 @@ module YearFieldAccessToLeapYearCheckFlow =
363341
predicate isYearModifiedWithCheck(YearFieldAccess fa) {
364342
exists(YearFieldAccessToLeapYearCheckFlow::PathNode src |
365343
src.isSource() and
366-
src.getNode().asExpr() = fa
344+
src.getNode().asExpr().(YearFieldAssignment).getYearFieldAccess() = fa
367345
)
368346
or
369347
isUsedInFeb29Check(fa)
@@ -385,6 +363,5 @@ import OperationToYearAssignmentFlow::PathGraph
385363
from OperationToYearAssignmentFlow::PathNode src, OperationToYearAssignmentFlow::PathNode sink
386364
where
387365
OperationToYearAssignmentFlow::flowPath(src, sink) and
388-
isRootOperationSource(src.getNode().asExpr()) and
389366
not isYearModifiedWithCheck(sink.getNode().asExpr().(YearFieldAssignment).getYearFieldAccess())
390367
select sink, src, sink, "TEST"

0 commit comments

Comments
 (0)