From a334dc9b2b551d81986c0ada3aaa6586871c796b Mon Sep 17 00:00:00 2001 From: Robert Marsh Date: Thu, 27 Oct 2022 15:06:48 -0400 Subject: [PATCH 1/2] C++: repair Adding365DaysPerYear.ql --- cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll | 21 ++++++++++++------- .../Adding365daysPerYear.expected | 5 +++++ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll b/cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll index 719c16281f2..c9d65b805d8 100644 --- a/cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll +++ b/cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll @@ -3,7 +3,7 @@ */ import cpp -import semmle.code.cpp.dataflow.DataFlow +import semmle.code.cpp.dataflow.TaintTracking import semmle.code.cpp.commons.DateTime /** @@ -248,24 +248,27 @@ class FiletimeYearArithmeticOperationCheckConfiguration extends DataFlow::Config /** * `DataFlow::Configuration` for finding an operation with hardcoded 365 that will flow into any known date/time field. */ -class PossibleYearArithmeticOperationCheckConfiguration extends DataFlow::Configuration { +class PossibleYearArithmeticOperationCheckConfiguration extends TaintTracking::Configuration { PossibleYearArithmeticOperationCheckConfiguration() { this = "PossibleYearArithmeticOperationCheckConfiguration" } override predicate isSource(DataFlow::Node source) { - exists(Operation op | op = source.asExpr() | + exists(Operation op | op = source.asConvertedExpr() | op.getAChild*().getValue().toInt() = 365 and - not op.getParent() instanceof Expr + ( + not op.getParent() instanceof Expr or + op.getParent() instanceof Assignment + ) ) } - override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { // flow from anything on the RHS of an assignment to a time/date structure to that // assignment. - exists(StructLikeClass dds, FieldAccess fa, AssignExpr aexpr, Expr e | + exists(StructLikeClass dds, FieldAccess fa, Assignment aexpr, Expr e | e = node1.asExpr() and - aexpr = node2.asExpr() + fa = node2.asExpr() | (dds instanceof PackedTimeType or dds instanceof UnpackedTimeType) and fa.getQualifier().getUnderlyingType() = dds and @@ -275,7 +278,9 @@ class PossibleYearArithmeticOperationCheckConfiguration extends DataFlow::Config } override predicate isSink(DataFlow::Node sink) { - exists(StructLikeClass dds, FieldAccess fa, AssignExpr aexpr | aexpr = sink.asExpr() | + exists(StructLikeClass dds, FieldAccess fa, AssignExpr aexpr | + aexpr.getRValue() = sink.asConvertedExpr() + | (dds instanceof PackedTimeType or dds instanceof UnpackedTimeType) and fa.getQualifier().getUnderlyingType() = dds and fa.isModified() and diff --git a/cpp/ql/test/query-tests/Likely Bugs/Leap Year/Adding365DaysPerYear/Adding365daysPerYear.expected b/cpp/ql/test/query-tests/Likely Bugs/Leap Year/Adding365DaysPerYear/Adding365daysPerYear.expected index e69de29bb2d..2d986e4b72f 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Leap Year/Adding365DaysPerYear/Adding365daysPerYear.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Leap Year/Adding365DaysPerYear/Adding365daysPerYear.expected @@ -0,0 +1,5 @@ +| test.cpp:173:29:173:51 | ... & ... | An arithmetic operation $@ that uses a constant value of 365 ends up modifying this date/time, without considering leap year scenarios. | test.cpp:170:16:170:47 | ... * ... | ... * ... | +| test.cpp:174:30:174:45 | ... >> ... | An arithmetic operation $@ that uses a constant value of 365 ends up modifying this date/time, without considering leap year scenarios. | test.cpp:170:16:170:47 | ... * ... | ... * ... | +| test.cpp:193:15:193:24 | ... / ... | An arithmetic operation $@ that uses a constant value of 365 ends up modifying this date/time, without considering leap year scenarios. | test.cpp:193:15:193:24 | ... / ... | ... / ... | +| test.cpp:217:29:217:51 | ... & ... | An arithmetic operation $@ that uses a constant value of 365 ends up modifying this date/time, without considering leap year scenarios. | test.cpp:214:16:214:47 | ... * ... | ... * ... | +| test.cpp:218:30:218:45 | ... >> ... | An arithmetic operation $@ that uses a constant value of 365 ends up modifying this date/time, without considering leap year scenarios. | test.cpp:214:16:214:47 | ... * ... | ... * ... | From f6ff9c9c663903bbd60b475456356340c248f23e Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 28 Oct 2022 14:32:08 +0200 Subject: [PATCH 2/2] Update cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll Co-authored-by: Geoffrey White <40627776+geoffw0@users.noreply.github.com> --- cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll b/cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll index c9d65b805d8..c758b956695 100644 --- a/cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll +++ b/cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll @@ -246,7 +246,7 @@ class FiletimeYearArithmeticOperationCheckConfiguration extends DataFlow::Config } /** - * `DataFlow::Configuration` for finding an operation with hardcoded 365 that will flow into any known date/time field. + * Taint configuration for finding an operation with hardcoded 365 that will flow into any known date/time field. */ class PossibleYearArithmeticOperationCheckConfiguration extends TaintTracking::Configuration { PossibleYearArithmeticOperationCheckConfiguration() {