Skip to content

Commit dfd85c3

Browse files
committed
C++: Compute 'IgnorableOperationToOperationSourceCandidateConfig' after an initial round of the query to reduce the number of sinks.
1 parent 8bbb0ec commit dfd85c3

1 file changed

Lines changed: 87 additions & 57 deletions

File tree

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

Lines changed: 87 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,30 @@ class IgnorableUnaryBitwiseOperation extends IgnorableOperation instanceof Unary
227227
class IgnorableAssignmentBitwiseOperation extends IgnorableOperation instanceof AssignBitwiseOperation
228228
{ }
229229

230+
class YearFieldAssignmentNode extends DataFlow::Node {
231+
YearFieldAccess access;
232+
233+
YearFieldAssignmentNode() {
234+
exists(Function f |
235+
f = this.getEnclosingCallable().getUnderlyingCallable() and not f instanceof IgnorableFunction
236+
|
237+
this.asDefinition().(Assignment).getLValue() = access
238+
or
239+
this.asDefinition().(CrementOperation).getOperand() = access
240+
or
241+
exists(Call c | c.getAnArgument() = access and this.asDefiningArgument() = access)
242+
or
243+
exists(Call c, AddressOfExpr aoe |
244+
c.getAnArgument() = aoe and
245+
aoe.getOperand() = access and
246+
this.asDefiningArgument() = aoe
247+
)
248+
)
249+
}
250+
251+
YearFieldAccess getYearFieldAccess() { result = access }
252+
}
253+
230254
/**
231255
* An arithmetic operation where one of the operands is a pointer or char type, ignore it
232256
*/
@@ -287,24 +311,7 @@ predicate isOperationSourceCandidate(Expr e) {
287311
}
288312

289313
/**
290-
* A data flow that tracks an ignorable operation (such as a bitwise operation) to an operation source, so we may disqualify it.
291-
*/
292-
module IgnorableOperationToOperationSourceCandidateConfig implements DataFlow::ConfigSig {
293-
predicate isSource(DataFlow::Node n) { n.asExpr() instanceof IgnorableOperation }
294-
295-
predicate isSink(DataFlow::Node n) { isOperationSourceCandidate(n.asExpr()) }
296-
297-
// looking for sources and sinks in the same function
298-
DataFlow::FlowFeature getAFeature() {
299-
result instanceof DataFlow::FeatureEqualSourceSinkCallContext
300-
}
301-
}
302-
303-
module IgnorableOperationToOperationSourceCandidateFlow =
304-
TaintTracking::Global<IgnorableOperationToOperationSourceCandidateConfig>;
305-
306-
/**
307-
* The set of all expressions which is a candidate expression and also does not flow from to to some ignorable expression (eg. bitwise op)
314+
* The set of all expressions which is a candidate expression.
308315
* ```
309316
* a = something <<< 2;
310317
* myDate.year = a + 1; // invalid
@@ -314,49 +321,16 @@ module IgnorableOperationToOperationSourceCandidateFlow =
314321
* ```
315322
*/
316323
class OperationSource extends Expr {
317-
OperationSource() {
318-
isOperationSourceCandidate(this) and
319-
// If the candidate came from an ignorable operation, ignore the candidate
320-
// NOTE: we cannot easily flow the candidate to an ignorable operation as that can
321-
// be tricky in practice, e.g., a mod operation on a year would be part of a leap year check
322-
// but a mod operation ending in a year is more indicative of something to ignore (a conversion)
323-
not exists(IgnorableOperationToOperationSourceCandidateFlow::PathNode sink |
324-
sink.getNode().asExpr() = this and
325-
sink.isSink()
326-
)
327-
}
328-
}
329-
330-
class YearFieldAssignmentNode extends DataFlow::Node {
331-
YearFieldAccess access;
332-
333-
YearFieldAssignmentNode() {
334-
exists(Function f |
335-
f = this.getEnclosingCallable().getUnderlyingCallable() and not f instanceof IgnorableFunction
336-
) and
337-
(
338-
this.asDefinition().(Assignment).getLValue() = access
339-
or
340-
this.asDefinition().(CrementOperation).getOperand() = access
341-
or
342-
exists(Call c | c.getAnArgument() = access and this.asDefiningArgument() = access)
343-
or
344-
exists(Call c, AddressOfExpr aoe |
345-
c.getAnArgument() = aoe and
346-
aoe.getOperand() = access and
347-
this.asDefiningArgument() = aoe
348-
)
349-
)
350-
}
351-
352-
YearFieldAccess getYearFieldAccess() { result = access }
324+
OperationSource() { isOperationSourceCandidate(this) }
353325
}
354326

355327
/**
356-
* A DataFlow configuration for identifying flows from an identified source
357-
* to the Year field of a date object.
328+
* An initial DataFlow configuration for identifying flows from an identified source
329+
* to the Year field of a date object. This is used to restrict the sinks of
330+
* `IgnorableOperationToOperationSourceCandidateConfig` and the sinks of the
331+
* final `OperationToYearAssignmentConfig`.
358332
*/
359-
module OperationToYearAssignmentConfig implements DataFlow::ConfigSig {
333+
module OperationToYearAssignmentConfig0 implements DataFlow::ConfigSig {
360334
predicate isSource(DataFlow::Node n) { n.asExpr() instanceof OperationSource }
361335

362336
predicate isSink(DataFlow::Node n) {
@@ -411,6 +385,62 @@ module OperationToYearAssignmentConfig implements DataFlow::ConfigSig {
411385
predicate isBarrierOut(DataFlow::Node n) { isSink(n) }
412386
}
413387

388+
module OperationToYearAssignmentFlow0 = TaintTracking::Global<OperationToYearAssignmentConfig0>;
389+
390+
predicate yearAssignmentFlowsFromSource(DataFlow::Node source, DataFlow::Node sink) {
391+
OperationToYearAssignmentFlow0::flow(source, sink)
392+
}
393+
394+
/**
395+
* A data flow that tracks an ignorable operation (such as a bitwise operation) to an operation source, so we may disqualify it.
396+
* Sinks are restricted to operation source candidates that have a flow to a year assignment in `OperationToYearAssignmentFlow0`.
397+
*/
398+
module IgnorableOperationToOperationSourceCandidateConfig implements DataFlow::ConfigSig {
399+
predicate isSource(DataFlow::Node n) { n.asExpr() instanceof IgnorableOperation }
400+
401+
predicate isSink(DataFlow::Node n) {
402+
isOperationSourceCandidate(n.asExpr()) and
403+
yearAssignmentFlowsFromSource(n, _)
404+
}
405+
406+
// looking for sources and sinks in the same function
407+
DataFlow::FlowFeature getAFeature() {
408+
result instanceof DataFlow::FeatureEqualSourceSinkCallContext
409+
}
410+
}
411+
412+
module IgnorableOperationToOperationSourceCandidateFlow =
413+
TaintTracking::Global<IgnorableOperationToOperationSourceCandidateConfig>;
414+
415+
/**
416+
* The final DataFlow configuration that refines `OperationToYearAssignmentConfig0` by
417+
* additionally filtering out operation sources that flow from an ignorable operation
418+
* (via `IgnorableOperationToOperationSourceCandidateFlow`).
419+
*/
420+
module OperationToYearAssignmentConfig implements DataFlow::ConfigSig {
421+
predicate isSource(DataFlow::Node n) { yearAssignmentFlowsFromSource(n, _) }
422+
423+
predicate isSink(DataFlow::Node n) {
424+
exists(DataFlow::Node operation |
425+
yearAssignmentFlowsFromSource(operation, n) and
426+
// If the candidate came from an ignorable operation, ignore the candidate
427+
// NOTE: we cannot easily flow the candidate to an ignorable operation as that can
428+
// be tricky in practice, e.g., a mod operation on a year would be part of a leap year check
429+
// but a mod operation ending in a year is more indicative of something to ignore (a conversion)
430+
not exists(IgnorableOperationToOperationSourceCandidateFlow::PathNode sink |
431+
sink.getNode() = operation and
432+
sink.isSink()
433+
)
434+
)
435+
}
436+
437+
predicate isBarrier(DataFlow::Node n) { OperationToYearAssignmentConfig0::isBarrier(n) }
438+
439+
predicate isBarrierIn(DataFlow::Node n) { isSource(n) }
440+
441+
predicate isBarrierOut(DataFlow::Node n) { isSink(n) }
442+
}
443+
414444
module OperationToYearAssignmentFlow = TaintTracking::Global<OperationToYearAssignmentConfig>;
415445

416446
predicate isLeapYearCheckSink(DataFlow::Node sink) {

0 commit comments

Comments
 (0)