More FP tweaks.

Author: bdrodes

cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll

@@ -534,7 +534,8 @@ class TimeConversionFunction extends Function {
         "FileTimeToSystemTime", "SystemTimeToFileTime", "SystemTimeToTzSpecificLocalTime",
         "SystemTimeToTzSpecificLocalTimeEx", "TzSpecificLocalTimeToSystemTime",
         "TzSpecificLocalTimeToSystemTimeEx", "RtlLocalTimeToSystemTime",
-        "RtlTimeToSecondsSince1970", "_mkgmtime", "SetSystemTime"
+        "RtlTimeToSecondsSince1970", "_mkgmtime", "SetSystemTime", "SystemTimeToVariantTime",
+        "VariantTimeToSystemTime"
       ]
   }
 }

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

@@ -16,9 +16,19 @@ import semmle.code.cpp.controlflow.IRGuards
 /**
  * Functions whose operations should never be considered a
  * source or sink of a dangerous leap year operation.
+ * The general concept is to add conversion functions
+ * that convert one time type to another. Often
+ * other ignorable operation heuristics will filter these,
+ * but some cases, the simplest approach is to simply filter
+ * the function entirely.
+ * Note that flow through these functions should still be allowed
+ * we just cannot start or end flow from an operation to a
+ * year assignment in one of these functions.
  */
 class IgnorableFunction extends Function {
   IgnorableFunction() {
+    this instanceof TimeConversionFunction
+    or
     // Helper utility in postgres with string time conversions
     this.getName() = "DecodeISO8601Interval"
     or
@@ -119,6 +129,9 @@ predicate isLikelyConversionConstant(int c) {
     i = 1899 or // Observed in uses with 1900 to address off by one scenarios
     i = 292275056 or // qdatetime.h Qt Core year range first year constant
     i = 292278994 or // qdatetime.h Qt Core year range last year constant
+    i = 1601 or // Windows FILETIME epoch start year
+    i = 1970 or // Unix epoch start year
+    i = 70 or // Unix epoch start year short form
     i = 0
   )
 }
@@ -351,9 +364,12 @@ module OperationToYearAssignmentConfig implements DataFlow::ConfigSig {
   predicate isBarrier(DataFlow::Node n) {
     exists(ArrayExpr arr | arr.getArrayOffset() = n.asExpr())
     or
-    n.asExpr().getUnspecifiedType() instanceof PointerType
+    n.getType().getUnspecifiedType() instanceof PointerType
+    or
+    n.getType().getUnspecifiedType() instanceof CharType
     or
-    n.asExpr().getUnspecifiedType() instanceof CharType
+    // If a type resembles "string" ignore flow (likely string conversion, currently ignored)
+    n.getType().getUnspecifiedType().stripType().getName().toLowerCase().matches("%string%")
     or
     n.asExpr() instanceof IgnorableOperation
     or

cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedLeapYearAfterYearModification.expected

@@ -22,6 +22,8 @@ edges
 | test.cpp:1083:2:1083:4 | ... ++ | test.cpp:1082:26:1082:26 | *x | provenance |  |
 | test.cpp:1086:37:1086:37 | *x | test.cpp:1099:27:1099:35 | increment_arg_by_pointer output argument | provenance |  |
 | test.cpp:1087:2:1087:7 | ... ++ | test.cpp:1086:37:1086:37 | *x | provenance |  |
+| test.cpp:1183:2:1183:15 | ... += ... | test.cpp:1185:16:1185:19 | year | provenance |  |
+| test.cpp:1183:10:1183:15 | offset | test.cpp:1183:2:1183:15 | ... += ... | provenance |  |
 nodes
 | test.cpp:422:14:422:14 | 2 | semmle.label | 2 |
 | test.cpp:440:2:440:11 | ... ++ | semmle.label | ... ++ |
@@ -56,6 +58,9 @@ nodes
 | test.cpp:1099:27:1099:35 | increment_arg_by_pointer output argument | semmle.label | increment_arg_by_pointer output argument |
 | test.cpp:1133:3:1133:15 | -- ... | semmle.label | -- ... |
 | test.cpp:1153:14:1153:26 | ... - ... | semmle.label | ... - ... |
+| test.cpp:1183:2:1183:15 | ... += ... | semmle.label | ... += ... |
+| test.cpp:1183:10:1183:15 | offset | semmle.label | offset |
+| test.cpp:1185:16:1185:19 | year | semmle.label | year |
 subpaths
 testFailures
 | test.cpp:1075:2:1075:11 | ... ++ | Unexpected result: Alert |

cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/test.cpp

@@ -1151,4 +1151,52 @@ typedef struct _TIME_FIELDS {
 void
 tp_ptime(PTIME_FIELDS ptm){
 	ptm->Year = ptm->Year - 1; // $ Alert[cpp/microsoft/public/leap-year/unchecked-after-arithmetic-year-modification]
+}
+
+
+bool isLeapYearRaw(WORD year)
+{
+	return year % 4 == 0 && (year % 100 != 0 || year % 400 == 0);
+}
+
+void leap_year_checked_raw_false_positive1(WORD year, WORD offset, WORD day){
+	struct tm tmp;
+
+	year += offset;
+
+	if (isLeapYearRaw(year)){
+		// in this simplified example, assume the logic of this function
+		// can assume a day is 28 by default
+		// this check is more to establish the leap year guard is present
+		day += 1;
+	}
+
+	// Assume the check handled leap year correctly
+	tmp.tm_year = year; // GOOD 
+	tmp.tm_mday = day;
+}
+
+
+void leap_year_checked_raw_false_positive2(WORD year, WORD offset, WORD day){
+	struct tm tmp;
+
+	year += offset;
+
+	tmp.tm_year = year; // GOOD, check performed immediately after on raw year
+
+	// Adding some additional checks to resemble cases observed in the wild
+	if ( day > 0)
+	{
+		if (isLeapYearRaw(year)){
+			// Assume logic that would adjust the day correctly
+		}
+	}
+	else{
+		if (isLeapYearRaw(year)){
+			// Assume logic that would adjust the day correctly
+		}
+	}
+
+	tmp.tm_mday = day;
+	
 }