Time conversion results that are checked in a ternary operator conditions are now assumed to be a check of the result, i.e., a valid leap year check.

Author: bdrodes

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

@@ -473,6 +473,13 @@ 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.
+ */
 module TimeStructToCheckedTimeConversionConfig implements DataFlow::StateConfigSig {
   class FlowState = boolean;
 
@@ -483,7 +490,11 @@ module TimeStructToCheckedTimeConversionConfig implements DataFlow::StateConfigS
 
   predicate isSink(DataFlow::Node sink, FlowState state) {
     state = true and
-    exists(IfStmt ifs | ifs.getCondition().getAChild*() = sink.asExpr())
+    (
+      exists(IfStmt ifs | ifs.getCondition().getAChild*() = sink.asExpr())
+      or
+      exists(ConditionalExpr ce | ce.getCondition().getAChild*() = sink.asExpr())
+    )
   }
 
   predicate isAdditionalFlowStep(

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

@@ -75,6 +75,8 @@ nodes
 | test.cpp:1243:16:1243:28 | ... + ... | semmle.label | ... + ... |
 | test.cpp:1250:16:1250:28 | ... + ... | semmle.label | ... + ... |
 | test.cpp:1257:16:1257:28 | ... + ... | semmle.label | ... + ... |
+| test.cpp:1263:16:1263:28 | ... + ... | semmle.label | ... + ... |
+| test.cpp:1277:14:1277:26 | ... + ... | semmle.label | ... + ... |
 subpaths
 testFailures
 | test.cpp:1075:2:1075:11 | ... ++ | Unexpected result: Alert |

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

@@ -1257,3 +1257,32 @@ void simplified_leap_year_check(WORD year, WORD offset){
 	tmp.tm_year = year + offset; // $ Alert[cpp/microsoft/public/leap-year/unchecked-after-arithmetic-year-modification]
 }
 
+void compound_leap_year_check(WORD year, WORD offset, WORD month, WORD day){
+	struct tm tmp;
+
+	tmp.tm_year = year + offset;
+
+	bool isLeap = tmp.tm_year % 4 == 0 && (tmp.tm_year % 100 != 0 || tmp.tm_year % 400 == 0) && (month == 2 && day == 29);
+	
+	if(isLeap){
+		// do something
+	}
+	tmp.tm_mday = day;
+	tmp.tm_mon = month;
+}
+
+void indirect_time_conversion_check(WORD year, WORD offset){
+	SYSTEMTIME tmp;
+
+	tmp.wYear = year + offset;
+
+	FILETIME ft;
+
+	// (out-of-scope) GeneralBug: Unchecked call to SystemTimeToFileTime. this may have failed, but we didn't check the return value!
+	BOOL res = SystemTimeToFileTime(&tmp, &ft);
+
+	// Assume this check of the result is sufficient as an implicit leap year check. 
+	bool x = (res == 0) ? true : false;
+}
+
+