Minor comment/description clean up, and added a test case and support for handing mktime and other variants that convert the time automatically without the need for a check if the date is an incorrect leap year date.

Author: bdrodes

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

@@ -528,18 +528,30 @@ class SafeTimeGatheringFunction extends Function {
  * This list of APIs should check for the return value to detect problems during the conversion.
  */
 class TimeConversionFunction extends Function {
+  boolean autoConverts;
+
   TimeConversionFunction() {
-    this.getQualifiedName() =
-      [
-        "FileTimeToSystemTime", "SystemTimeToFileTime", "SystemTimeToTzSpecificLocalTime",
-        "SystemTimeToTzSpecificLocalTimeEx", "TzSpecificLocalTimeToSystemTime",
-        "TzSpecificLocalTimeToSystemTimeEx", "RtlLocalTimeToSystemTime",
-        "RtlTimeToSecondsSince1970", "_mkgmtime", "SetSystemTime", "SystemTimeToVariantTime",
-        "VariantTimeToSystemTime",
-        // NOTE: mktime will normalize a Feb 29 on a non-leap year to Mar 1 silently,
-        "mktime", "_mktime32", "_mktime64", "VarUdateFromDate"
-      ]
+    autoConverts = false and
+    (
+      this.getQualifiedName() =
+        [
+          "FileTimeToSystemTime", "SystemTimeToFileTime", "SystemTimeToTzSpecificLocalTime",
+          "SystemTimeToTzSpecificLocalTimeEx", "TzSpecificLocalTimeToSystemTime",
+          "TzSpecificLocalTimeToSystemTimeEx", "RtlLocalTimeToSystemTime",
+          "RtlTimeToSecondsSince1970", "_mkgmtime", "SetSystemTime", "SystemTimeToVariantTime",
+          "VariantTimeToSystemTime", "VarUdateFromDate", "boost::gregorian::date::from_tm"
+        ]
+      or
+      this.getQualifiedName().matches("GetDateFormat%")
+    )
     or
-    this.getQualifiedName().matches("GetDateFormat%")
+    // NOTE: mktime will normalize a Feb 29 on a non-leap year to Mar 1 silently,
+    autoConverts = true and
+    this.getQualifiedName() = ["mktime", "_mktime32", "_mktime64"]
   }
+
+  /**
+   * Holds if the function is expected to auto convert a bad leap year date.
+   */
+  predicate isAutoConverting() { autoConverts = true }
 }

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

@@ -1,6 +1,6 @@
 /**
  * @name Year field changed using an arithmetic operation without checking for leap year (AntiPattern 1)
- * @description A field that represents a year is being modified by an arithmetic operation, but no proper check for leap years can be detected afterwards.
+ * @description A field that represents a year is being modified by an arithmetic operation, but no proper check for leap years can be detected.
  * @kind path-problem
  * @problem.severity warning
  * @id cpp/microsoft/public/leap-year/unchecked-after-arithmetic-year-modification
@@ -76,6 +76,10 @@ class IgnorableFunction extends Function {
     this.getName()
         .toLowerCase()
         .matches(["%char%to%", "%string%to%", "%from%char%", "%from%string%"])
+    or
+    // boost's gregorian.cpp has year manipulations that are checked in complex ways.
+    // ignore the entire file as a source or sink.
+    this.getFile().getAbsolutePath().toLowerCase().matches("%boost%gregorian.cpp%")
   }
 }
 
@@ -490,11 +494,9 @@ predicate isControlledByMonthEqualityCheckNonFebruary(Expr e) {
 }
 
 /**
- * Flow from a year field access through a time conversion function
- * where the call's result is used to check error. The result must
- * be used as a guard for an if or ternary operator. If so,
- * assume some sort of error handling is occurring that could be used
- * to detect bad dates due to leap year.
+ * From from a year field access to a time conversion function
+ * that auto converts feb29 in non-leap year, or through a conversion function that doesn't
+ * auto convert to a sanity check guard of the result for error conditions.
  */
 module YearAssignmentToCheckedTimeConversionConfig implements DataFlow::StateConfigSig {
   class FlowState = boolean;
@@ -515,6 +517,13 @@ module YearAssignmentToCheckedTimeConversionConfig implements DataFlow::StateCon
       or
       exists(Loop l | l.getCondition().getAChild*() = [sink.asExpr(), sink.asIndirectExpr()])
     )
+    or
+    state in [true, false] and
+    exists(Call c, TimeConversionFunction f |
+      f.isAutoConverting() and
+      c.getTarget() = f and
+      c.getAnArgument().getAChild*() = [sink.asExpr(), sink.asIndirectExpr()]
+    )
   }
 
   predicate isAdditionalFlowStep(

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

@@ -159,4 +159,7 @@ nodes
 | test.cpp:1584:2:1584:22 | ... += ... | semmle.label | ... += ... |
 | test.cpp:1596:2:1596:22 | ... += ... | semmle.label | ... += ... |
 | test.cpp:1609:2:1609:22 | ... += ... | semmle.label | ... += ... |
+| test.cpp:1644:2:1644:22 | ... += ... | semmle.label | ... += ... |
+| test.cpp:1649:2:1649:22 | ... += ... | semmle.label | ... += ... |
+| test.cpp:1678:2:1678:22 | ... += ... | semmle.label | ... += ... |
 subpaths

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

@@ -1671,5 +1671,13 @@ void leap_year_check_call_on_conversion3(tm timeinfo, WORD year, WORD month, WOR
 	timeinfo.tm_year = year;
 }
 
+void assumed_maketime_conversion1(tm timeinfo)
+{
+	//the docs of mktime suggest feb29 is handled, and conversion will occur automatically
+	//no check required.
+	timeinfo.tm_year += 1;
+
+	mktime(&timeinfo);
+}