From 6c171c804f5d4765ef2bf265205442eb58875f42 Mon Sep 17 00:00:00 2001 From: "REDMOND\\brodes" Date: Fri, 6 Feb 2026 15:20:52 -0500 Subject: [PATCH 1/8] C++: Add more tests for modified years with and without leap year checks (UncheckedLeapYearAfterYearModification). Switch to using 'postprocess' for unit tests. --- ...ckedLeapYearAfterYearModification.expected | 75 +- ...checkedLeapYearAfterYearModification.qlref | 3 +- ...heckedReturnValueForTimeFunctions.expected | 11 +- .../test.cpp | 1257 +++++++++++++++-- 4 files changed, 1232 insertions(+), 114 deletions(-) diff --git a/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedLeapYearAfterYearModification.expected b/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedLeapYearAfterYearModification.expected index a9c1bc66c50f..8fdc1339aa1e 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedLeapYearAfterYearModification.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedLeapYearAfterYearModification.expected @@ -1,15 +1,60 @@ -| test.cpp:314:5:314:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:309:13:309:14 | st | st | -| test.cpp:327:5:327:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:322:13:322:14 | st | st | -| test.cpp:338:6:338:10 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:333:62:333:63 | st | st | -| test.cpp:484:5:484:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:480:13:480:14 | st | st | -| test.cpp:497:5:497:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:492:13:492:14 | st | st | -| test.cpp:509:5:509:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:505:13:505:14 | st | st | -| test.cpp:606:11:606:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:56:6:56:12 | tm_year | tm_year | test.cpp:602:12:602:19 | timeinfo | timeinfo | -| test.cpp:634:11:634:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:56:6:56:12 | tm_year | tm_year | test.cpp:628:12:628:19 | timeinfo | timeinfo | -| test.cpp:636:11:636:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:56:6:56:12 | tm_year | tm_year | test.cpp:628:12:628:19 | timeinfo | timeinfo | -| test.cpp:640:5:640:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:629:13:629:14 | st | st | -| test.cpp:642:5:642:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:629:13:629:14 | st | st | -| test.cpp:718:5:718:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:712:13:712:14 | st | st | -| test.cpp:731:5:731:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:725:13:725:14 | st | st | -| test.cpp:732:5:732:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:725:13:725:14 | st | st | -| test.cpp:733:5:733:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:725:13:725:14 | st | st | +#select +| test.cpp:422:5:422:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:417:13:417:14 | st | st | +| test.cpp:440:5:440:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:435:13:435:14 | st | st | +| test.cpp:456:6:456:10 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:451:62:451:63 | st | st | +| test.cpp:647:5:647:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:643:13:643:14 | st | st | +| test.cpp:665:5:665:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:661:13:661:14 | st | st | +| test.cpp:681:5:681:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:677:13:677:14 | st | st | +| test.cpp:792:11:792:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:789:12:789:19 | timeinfo | timeinfo | +| test.cpp:813:11:813:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:806:12:806:19 | timeinfo | timeinfo | +| test.cpp:818:5:818:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:807:13:807:14 | st | st | +| test.cpp:954:6:954:10 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:947:14:947:15 | st | st | +| test.cpp:972:6:972:10 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:965:14:965:15 | st | st | +| test.cpp:990:6:990:10 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:984:14:984:15 | st | st | +| test.cpp:1077:5:1077:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:1071:13:1071:14 | st | st | +| test.cpp:1135:9:1135:15 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:92:17:92:23 | tm_year | tm_year | test.cpp:1126:35:1126:36 | tm | tm | +| test.cpp:1137:9:1137:15 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:92:17:92:23 | tm_year | tm_year | test.cpp:1126:35:1126:36 | tm | tm | +| test.cpp:1591:11:1591:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1590:41:1590:48 | timeinfo | timeinfo | +| test.cpp:1599:11:1599:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1598:41:1598:48 | timeinfo | timeinfo | +| test.cpp:1645:11:1645:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1644:57:1644:64 | timeinfo | timeinfo | +| test.cpp:1656:11:1656:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1655:57:1655:64 | timeinfo | timeinfo | +| test.cpp:1667:11:1667:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1666:30:1666:37 | timeinfo | timeinfo | +| test.cpp:1678:11:1678:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1677:30:1677:37 | timeinfo | timeinfo | +| test.cpp:1690:11:1690:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1689:30:1689:37 | timeinfo | timeinfo | +| test.cpp:1702:11:1702:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1701:30:1701:37 | timeinfo | timeinfo | +| test.cpp:1712:11:1712:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1711:30:1711:37 | timeinfo | timeinfo | +| test.cpp:1723:11:1723:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1722:40:1722:47 | timeinfo | timeinfo | +| test.cpp:1758:11:1758:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1757:45:1757:52 | timeinfo | timeinfo | +| test.cpp:1763:11:1763:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1762:45:1762:52 | timeinfo | timeinfo | +| test.cpp:1792:11:1792:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1788:38:1788:45 | timeinfo | timeinfo | +| test.cpp:1799:11:1799:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1798:36:1798:43 | timeinfo | timeinfo | +testFailures +| test.cpp:422:5:422:9 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:440:5:440:9 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:456:6:456:10 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:647:5:647:9 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:665:5:665:9 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:681:5:681:9 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:792:11:792:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:813:11:813:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:818:5:818:9 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:954:6:954:10 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:972:6:972:10 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:990:6:990:10 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:1077:5:1077:9 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:1135:9:1135:15 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:1137:9:1137:15 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:1591:11:1591:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:1599:11:1599:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:1645:11:1645:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:1656:11:1656:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:1667:11:1667:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:1678:11:1678:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:1690:11:1690:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:1702:11:1702:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:1712:11:1712:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:1723:11:1723:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:1758:11:1758:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:1763:11:1763:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:1792:11:1792:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:1799:11:1799:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedLeapYearAfterYearModification.qlref b/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedLeapYearAfterYearModification.qlref index 22a30d72b689..12bb5eb1e69b 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedLeapYearAfterYearModification.qlref +++ b/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedLeapYearAfterYearModification.qlref @@ -1 +1,2 @@ -Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql +query: Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql \ No newline at end of file diff --git a/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedReturnValueForTimeFunctions.expected b/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedReturnValueForTimeFunctions.expected index fb79592b7f2d..e893ae1fff0e 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedReturnValueForTimeFunctions.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedReturnValueForTimeFunctions.expected @@ -1,5 +1,6 @@ -| test.cpp:317:2:317:21 | call to SystemTimeToFileTime | Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe. | test.cpp:63:1:63:20 | SystemTimeToFileTime | SystemTimeToFileTime | test.cpp:309:13:309:14 | st | st | -| test.cpp:330:2:330:21 | call to SystemTimeToFileTime | Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe. | test.cpp:63:1:63:20 | SystemTimeToFileTime | SystemTimeToFileTime | test.cpp:322:13:322:14 | st | st | -| test.cpp:341:2:341:21 | call to SystemTimeToFileTime | Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe. | test.cpp:63:1:63:20 | SystemTimeToFileTime | SystemTimeToFileTime | test.cpp:333:62:333:63 | st | st | -| test.cpp:720:2:720:21 | call to SystemTimeToFileTime | Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe. | test.cpp:63:1:63:20 | SystemTimeToFileTime | SystemTimeToFileTime | test.cpp:712:13:712:14 | st | st | -| test.cpp:735:2:735:21 | call to SystemTimeToFileTime | Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe. | test.cpp:63:1:63:20 | SystemTimeToFileTime | SystemTimeToFileTime | test.cpp:725:13:725:14 | st | st | +| test.cpp:425:2:425:21 | call to SystemTimeToFileTime | Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe. | test.cpp:101:1:101:20 | SystemTimeToFileTime | SystemTimeToFileTime | test.cpp:417:13:417:14 | st | st | +| test.cpp:443:2:443:21 | call to SystemTimeToFileTime | Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe. | test.cpp:101:1:101:20 | SystemTimeToFileTime | SystemTimeToFileTime | test.cpp:435:13:435:14 | st | st | +| test.cpp:459:2:459:21 | call to SystemTimeToFileTime | Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe. | test.cpp:101:1:101:20 | SystemTimeToFileTime | SystemTimeToFileTime | test.cpp:451:62:451:63 | st | st | +| test.cpp:956:3:956:22 | call to SystemTimeToFileTime | Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe. | test.cpp:101:1:101:20 | SystemTimeToFileTime | SystemTimeToFileTime | test.cpp:947:14:947:15 | st | st | +| test.cpp:974:3:974:22 | call to SystemTimeToFileTime | Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe. | test.cpp:101:1:101:20 | SystemTimeToFileTime | SystemTimeToFileTime | test.cpp:965:14:965:15 | st | st | +| test.cpp:1081:2:1081:21 | call to SystemTimeToFileTime | Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe. | test.cpp:101:1:101:20 | SystemTimeToFileTime | SystemTimeToFileTime | test.cpp:1071:13:1071:14 | st | st | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/test.cpp b/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/test.cpp index 3db9b61edd2b..4451450fb254 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/test.cpp +++ b/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/test.cpp @@ -46,6 +46,8 @@ typedef struct _TIME_DYNAMIC_ZONE_INFORMATION { BOOLEAN DynamicDaylightTimeDisabled; } DYNAMIC_TIME_ZONE_INFORMATION, *PDYNAMIC_TIME_ZONE_INFORMATION; +typedef const wchar_t* LPCWSTR; + struct tm { int tm_sec; // seconds after the minute - [0, 60] including leap second @@ -59,6 +61,42 @@ struct tm int tm_isdst; // daylight savings time flag }; +struct timespec +{ + time_t tv_sec; + long tv_nsec; +}; + +/* Timestamps of log entries. */ +struct logtime { + struct tm tm; + long usec; +}; + +/* + * Data structure representing a broken-down timestamp. + * + * CAUTION: the IANA timezone library (src/timezone/) follows the POSIX + * convention that tm_mon counts from 0 and tm_year is relative to 1900. + * However, Postgres' datetime functions generally treat tm_mon as counting + * from 1 and tm_year as relative to 1 BC. Be sure to make the appropriate + * adjustments when moving from one code domain to the other. + */ +struct pg_tm +{ + int tm_sec; + int tm_min; + int tm_hour; + int tm_mday; + int tm_mon; /* see above */ + int tm_year; /* see above */ + int tm_wday; + int tm_yday; + int tm_isdst; + long int tm_gmtoff; + const char *tm_zone; +}; + BOOL SystemTimeToFileTime( const SYSTEMTIME* lpSystemTime, @@ -99,9 +137,16 @@ TzSpecificLocalTimeToSystemTimeEx( LPSYSTEMTIME lpUniversalTime ); +int _wtoi( + const wchar_t *str +); + void GetSystemTime( LPSYSTEMTIME lpSystemTime ); +void GetLocalTime( + LPSYSTEMTIME lpSystemTime +); void GetSystemTimeAsFileTime( LPFILETIME lpSystemTimeAsFileTime @@ -149,6 +194,12 @@ GetFileTime( LPFILETIME lpLastWriteTime ); +struct tm *localtime_r( const time_t *timer, struct tm *buf ); + +/** + * Negative Case + * FileTimeToSystemTime is called and the return value is checked +*/ void Correct_FileTimeToSystemTime(const FILETIME* lpFileTime) { SYSTEMTIME systemTime; @@ -162,6 +213,10 @@ void Correct_FileTimeToSystemTime(const FILETIME* lpFileTime) /// Normal usage } +/** + * Positive (Out of Scope) Bug Case + * FileTimeToSystemTime is called but no check is conducted to verify the result of the operation +*/ void AntiPattern_FileTimeToSystemTime(const FILETIME* lpFileTime) { SYSTEMTIME systemTime; @@ -170,6 +225,10 @@ void AntiPattern_FileTimeToSystemTime(const FILETIME* lpFileTime) FileTimeToSystemTime(lpFileTime, &systemTime); } +/** + * Negative Case + * SystemTimeToTzSpecificLocalTime is called and the return value is verified +*/ void Correct_SystemTimeToTzSpecificLocalTime(const TIME_ZONE_INFORMATION *lpTimeZoneInformation, const SYSTEMTIME *lpUniversalTime) { SYSTEMTIME localTime; @@ -183,6 +242,10 @@ void Correct_SystemTimeToTzSpecificLocalTime(const TIME_ZONE_INFORMATION *lpTime /// Normal usage } +/** + * Positive (Out of Scope) Case + * AntiPattern_SystemTimeToTzSpecificLocalTime is called but the return value is not validated +*/ void AntiPattern_SystemTimeToTzSpecificLocalTime(const TIME_ZONE_INFORMATION *lpTimeZoneInformation, const SYSTEMTIME *lpUniversalTime) { SYSTEMTIME localTime; @@ -191,6 +254,10 @@ void AntiPattern_SystemTimeToTzSpecificLocalTime(const TIME_ZONE_INFORMATION *lp SystemTimeToTzSpecificLocalTime(lpTimeZoneInformation, lpUniversalTime, &localTime); } +/** + * Negative Case + * SystemTimeToTzSpecificLocalTimeEx is called and the return value is validated +*/ void Correct_SystemTimeToTzSpecificLocalTimeEx(const DYNAMIC_TIME_ZONE_INFORMATION *lpTimeZoneInformation, const SYSTEMTIME *lpUniversalTime) { SYSTEMTIME localTime; @@ -204,6 +271,10 @@ void Correct_SystemTimeToTzSpecificLocalTimeEx(const DYNAMIC_TIME_ZONE_INFORMATI /// Normal usage } +/** + * Positive Case + * SystemTimeToTzSpecificLocalTimeEx is called but the return value is not validated +*/ void AntiPattern_SystemTimeToTzSpecificLocalTimeEx(const DYNAMIC_TIME_ZONE_INFORMATION *lpTimeZoneInformation, const SYSTEMTIME *lpUniversalTime) { SYSTEMTIME localTime; @@ -212,6 +283,10 @@ void AntiPattern_SystemTimeToTzSpecificLocalTimeEx(const DYNAMIC_TIME_ZONE_INFOR SystemTimeToTzSpecificLocalTimeEx(lpTimeZoneInformation, lpUniversalTime, &localTime); } +/** + * Negative Case + * Correct use of TzSpecificLocalTimeToSystemTime, function is called and the return value is validated. +*/ void Correct_TzSpecificLocalTimeToSystemTime(const TIME_ZONE_INFORMATION *lpTimeZoneInformation, const SYSTEMTIME *lpLocalTime) { SYSTEMTIME universalTime; @@ -225,6 +300,10 @@ void Correct_TzSpecificLocalTimeToSystemTime(const TIME_ZONE_INFORMATION *lpTime /// Normal usage } +/** + * Positive (Out of Scope) Case + * TzSpecificLocalTimeToSystemTime is called however the return value is not validated +*/ void AntiPattern_TzSpecificLocalTimeToSystemTime(const TIME_ZONE_INFORMATION *lpTimeZoneInformation, const SYSTEMTIME *lpLocalTime) { SYSTEMTIME universalTime; @@ -233,6 +312,10 @@ void AntiPattern_TzSpecificLocalTimeToSystemTime(const TIME_ZONE_INFORMATION *lp TzSpecificLocalTimeToSystemTime(lpTimeZoneInformation, lpLocalTime, &universalTime); } +/** + * Negative Case + * TzSpecificLocalTimeToSystemTimeEx is called and the return value is correctly validated +*/ void Correct_TzSpecificLocalTimeToSystemTimeEx(const DYNAMIC_TIME_ZONE_INFORMATION *lpTimeZoneInformation, const SYSTEMTIME *lpLocalTime) { SYSTEMTIME universalTime; @@ -246,6 +329,10 @@ void Correct_TzSpecificLocalTimeToSystemTimeEx(const DYNAMIC_TIME_ZONE_INFORMATI /// Normal usage } +/** + * Positive (Out of Scope) Case + * TzSpecificLocalTimeToSystemTimeEx is called however the return value is not validated +*/ void AntiPattern_TzSpecificLocalTimeToSystemTimeEx(const DYNAMIC_TIME_ZONE_INFORMATION *lpTimeZoneInformation, const SYSTEMTIME *lpLocalTime) { SYSTEMTIME universalTime; @@ -258,6 +345,10 @@ void AntiPattern_TzSpecificLocalTimeToSystemTimeEx(const DYNAMIC_TIME_ZONE_INFOR SYSTEMTIME Cases *************************************************/ +/** + * Negative Case + * SystemTimeToFileTime is called and the return value is validated in a guard +*/ void Correct_filetime_conversion_check(SYSTEMTIME& st) { FILETIME ft; @@ -273,6 +364,10 @@ void Correct_filetime_conversion_check(SYSTEMTIME& st) ////////////////////////////////////////////// +/** + * Positive (Out of Scope) Case + * SystemTimeToFileTime is called but the return value is not validated in a guard +*/ void AntiPattern_unchecked_filetime_conversion(SYSTEMTIME& st) { FILETIME ft; @@ -281,6 +376,10 @@ void AntiPattern_unchecked_filetime_conversion(SYSTEMTIME& st) SystemTimeToFileTime(&st, &ft); } +/** + * Positive (Out of Scope) Case + * SystemTimeToFileTime is called but the return value is not validated in a guard +*/ void AntiPattern_unchecked_filetime_conversion2(SYSTEMTIME* st) { FILETIME ft; @@ -292,6 +391,10 @@ void AntiPattern_unchecked_filetime_conversion2(SYSTEMTIME* st) } } +/** + * Positive (Out of Scope) + * SYSTEMTIME.wDay is incremented by one (and no guard exists) +*/ void AntiPattern_unchecked_filetime_conversion2() { SYSTEMTIME st; @@ -304,6 +407,11 @@ void AntiPattern_unchecked_filetime_conversion2() SystemTimeToFileTime(&st, &ft); } +/** + * Positive Cases + * - Anti-pattern 1: [year ±n, month, day] + * - Generic (Out of Scope) - UncheckedReturnValueForTimeFunctions +*/ void AntiPattern_unchecked_filetime_conversion2a() { SYSTEMTIME st; @@ -311,12 +419,17 @@ void AntiPattern_unchecked_filetime_conversion2a() GetSystemTime(&st); // BUG - UncheckedLeapYearAfterYearModification - st.wYear += 2; + st.wYear += 2; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] // BUG - UncheckedReturnValueForTimeFunctions SystemTimeToFileTime(&st, &ft); } +/** + * Positive Cases + * - Anti-pattern 1: [year ±n, month, day] + * - Generic (Out of Scope) - UncheckedReturnValueForTimeFunctions +*/ void AntiPattern_unchecked_filetime_conversion2b() { SYSTEMTIME st; @@ -324,23 +437,33 @@ void AntiPattern_unchecked_filetime_conversion2b() GetSystemTime(&st); // BUG - UncheckedLeapYearAfterYearModification - st.wYear++; + st.wYear++; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] // BUG - UncheckedReturnValueForTimeFunctions SystemTimeToFileTime(&st, &ft); } +/** + * Positive Cases + * - Anti-pattern 1: [year ±n, month, day] + * - Generic (Out of Scope) - UncheckedReturnValueForTimeFunctions +*/ void AntiPattern_unchecked_filetime_conversion2b(SYSTEMTIME* st) { FILETIME ft; // BUG - UncheckedLeapYearAfterYearModification - st->wYear++; + st->wYear++; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] // BUG - UncheckedReturnValueForTimeFunctions SystemTimeToFileTime(st, &ft); } +/** + * Positive Cases + * - Anti-pattern 3: datetime.AddDays(±28) + * - Generic (Out of Scope) - UncheckedReturnValueForTimeFunctions +*/ void AntiPattern_unchecked_filetime_conversion3() { SYSTEMTIME st; @@ -349,11 +472,12 @@ void AntiPattern_unchecked_filetime_conversion3() if (st.wMonth < 12) { + // Anti-pattern 3: datetime.AddDays(±28) st.wMonth++; } else { - // Check for leap year, but... + // No check for leap year is required here, as the month is statically set to January. st.wMonth = 1; st.wYear++; } @@ -363,6 +487,11 @@ void AntiPattern_unchecked_filetime_conversion3() } ////////////////////////////////////////////// + +/** + * Negative Case - Anti-pattern 1: [year ±n, month, day] + * Year is incremented and if we are on Feb the 29th, set to the 28th if the new year is a common year. +*/ void CorrectPattern_check1() { SYSTEMTIME st; @@ -370,7 +499,7 @@ void CorrectPattern_check1() st.wYear++; - // Guard + // Guard against February the 29th if (st.wMonth == 2 && st.wDay == 29) { // move back a day when landing on Feb 29 in an non-leap year @@ -385,6 +514,10 @@ void CorrectPattern_check1() AntiPattern_unchecked_filetime_conversion(st); } +/** + * Negative Case - Anti-pattern 1: [year ±n, month, day] + * Years is incremented by some integer and then the leap year case is correctly guarded and handled. +*/ void CorrectPattern_check2(int yearsToAdd) { SYSTEMTIME st; @@ -400,11 +533,18 @@ void CorrectPattern_check2(int yearsToAdd) AntiPattern_unchecked_filetime_conversion(st); } +/** + * Could give rise to AntiPattern 7: IsLeapYear (Conditional Logic) +*/ bool isLeapYear(SYSTEMTIME& st) { return st.wYear % 4 == 0 && (st.wYear % 100 != 0 || st.wYear % 400 == 0); } +/** + * Negative Case - Anti-pattern 1: [year ±n, month, day] + * Years is incremented by some integer and then the leap year case is correctly guarded and handled. +*/ void CorrectPattern_check3() { SYSTEMTIME st; @@ -413,6 +553,9 @@ void CorrectPattern_check3() st.wYear++; // Guard + /** Negative Case - Anti-pattern 7: IsLeapYear + * Body of conditional statement is safe recommended code + */ if (st.wMonth == 2 && st.wDay == 29 && isLeapYear(st)) { // move back a day when landing on Feb 29 in an non-leap year @@ -423,6 +566,9 @@ void CorrectPattern_check3() AntiPattern_unchecked_filetime_conversion(st); } +/** + * Could give rise to AntiPattern 7: IsLeapYear (Conditional Logic) +*/ bool isLeapYear2(int year) { return year % 4 == 0 && (year % 100 != 0 || year % 400 == 0); @@ -433,6 +579,10 @@ bool fixDate(int day, int month, int year) return (month == 2 && day == 29 && isLeapYear2(year)); } +/** + * Negative Case - Anti-pattern 1: [year ±n, month, day] + * Years is incremented by some integer and then the leap year case is correctly guarded and handled. +*/ void CorrectPattern_check4() { SYSTEMTIME st; @@ -442,18 +592,23 @@ void CorrectPattern_check4() st.wYear++; // Guard + /** Negative Case - Anti-pattern 7: IsLeapYear + * Body of conditional statement is safe recommended code + */ if (fixDate(st.wDay, st.wMonth, st.wYear)) { // move back a day when landing on Feb 29 in an non-leap year - st.wDay = 28; // GOOD [FALSE POSITIVE] + st.wDay = 28; // GOOD [FALSE POSITIVE] Anti-pattern 7 } // Safe to use AntiPattern_unchecked_filetime_conversion(st); } - - +/** + * Negative Case - Generic + * No manipulation is conducted on struct populated from GetSystemTime. +*/ void CorrectPattern_NotManipulated_DateFromAPI_0() { SYSTEMTIME st; @@ -464,6 +619,10 @@ void CorrectPattern_NotManipulated_DateFromAPI_0() SystemTimeToFileTime(&st, &ft); } +/** + * Negative Case - Generic + * No manipulation is conducted on struct populated from GetFileTime. +*/ void CorrectPattern_NotManipulated_DateFromAPI_1(HANDLE hWatchdog) { SYSTEMTIME st; @@ -475,43 +634,56 @@ void CorrectPattern_NotManipulated_DateFromAPI_1(HANDLE hWatchdog) ///////////////////////////////////////////////////////////////// +/** + * Negative Case - Anti-pattern 1: [year ±n, month, day] + * Year is incremented by some integer and checked through a conversion through an inter procedural function check +*/ void AntiPattern_1_year_addition() { SYSTEMTIME st; GetSystemTime(&st); - // BUG - UncheckedLeapYearAfterYearModification - st.wYear++; + // Safe, checked interprocedurally through Correct_filetime_conversion_check + st.wYear++; // Usage of potentially invalid date Correct_filetime_conversion_check(st); } + + +/** + * Negative Case - Anti-pattern 1: [year ±n, month, day] + * Years is incremented by some integer and checked through a conversion through an inter procedural function check +*/ void AntiPattern_simple_addition(int yearAddition) { SYSTEMTIME st; GetSystemTime(&st); - // BUG - UncheckedLeapYearAfterYearModification - st.wYear += yearAddition; + st.wYear += yearAddition; // Usage of potentially invalid date Correct_filetime_conversion_check(st); } +/** + * Positive Case - Anti-pattern 1: [year ±n, month, day] + * Years is incremented by some integer but a leap year is not handled *correctly*. +*/ void AntiPattern_IncorrectGuard(int yearsToAdd) { SYSTEMTIME st; GetSystemTime(&st); // BUG - UncheckedLeapYearAfterYearModification - st.wYear += yearsToAdd; + st.wYear += yearsToAdd; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] // Incorrect Guard if (st.wMonth == 2 && st.wDay == 29) { - // Part of a different anti-pattern. + // Part of a different anti-pattern (AntiPattern 5). // Make sure the guard includes the proper check bool isLeapYear = st.wYear % 4 == 0; if (!isLeapYear) @@ -519,9 +691,6 @@ void AntiPattern_IncorrectGuard(int yearsToAdd) st.wDay = 28; } } - - // Potentially Unsafe to use - Correct_filetime_conversion_check(st); } /************************************************* @@ -539,6 +708,10 @@ void CorrectUsageOf_mkgmtime(struct tm& timeinfo) /// _mkgmtime succeeded } +/** + * Positive Case - General (Out of Scope) + * Must Check for return value of _mkgmtime +*/ void AntiPattern_uncheckedUsageOf_mkgmtime(struct tm& timeinfo) { // (out-of-scope) GeneralBug: Must check return value for _mkgmtime @@ -550,6 +723,10 @@ void AntiPattern_uncheckedUsageOf_mkgmtime(struct tm& timeinfo) ////////////////////////////////////////////////////////// +/** + * Negative Case - Anti-pattern 1: [year ±n, month, day] + * Years is incremented by some integer and leap year is not handled correctly. +*/ void Correct_year_addition_struct_tm() { time_t rawtime; @@ -575,7 +752,11 @@ void Correct_year_addition_struct_tm() AntiPattern_uncheckedUsageOf_mkgmtime(timeinfo); } -void Correct_LinuxPattern() +/** + * Positive Case - Anti-pattern 1: [year ±n, month, day] + * Years is incremented by some integer and leap year is not handled correctly. +*/ +void Incorrect_LinuxPattern() { time_t rawtime; struct tm timeinfo; @@ -584,7 +765,8 @@ void Correct_LinuxPattern() errno_t err = gmtime_s(&timeinfo, &rawtime); /* from 1900 -> from 1980 */ - timeinfo.tm_year -= 80; + // BUG - UncheckedLeapYearAfterYearModification + timeinfo.tm_year -= 80; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] /* 0~11 -> 1~12 */ timeinfo.tm_mon++; /* 0~59 -> 0~29(2sec counts) */ @@ -596,34 +778,30 @@ void Correct_LinuxPattern() ////////////////////////////////////////// +/** + * Negative Case - Anti-pattern 1: [year ±n, month, day] + * Years is incremented by some integer and leap year is assumed checked through + * check of a conversion functions return value. +*/ void AntiPattern_year_addition_struct_tm() { time_t rawtime; struct tm timeinfo; time(&rawtime); gmtime_s(&timeinfo, &rawtime); - // BUG - UncheckedLeapYearAfterYearModification - timeinfo.tm_year++; + timeinfo.tm_year++; - // Usage of potentially invalid date + // mkgmtime result checked in nested call here, assume leap year conversion is potentially handled CorrectUsageOf_mkgmtime(timeinfo); } ///////////////////////////////////////////////////////// -void FalsePositiveTests(int x) -{ - struct tm timeinfo; - SYSTEMTIME st; - - timeinfo.tm_year = x; - timeinfo.tm_year = 1970; - - st.wYear = x; - st.wYear = 1900 + x; -} -void FalseNegativeTests(int x) +/** + * Positive Case - Anti-pattern 1: [year ±n, month, day] +*/ +void test(int x) { struct tm timeinfo; SYSTEMTIME st; @@ -631,106 +809,999 @@ void FalseNegativeTests(int x) timeinfo.tm_year = x; // BUG - UncheckedLeapYearAfterYearModification - timeinfo.tm_year = x + timeinfo.tm_year; - // BUG - UncheckedLeapYearAfterYearModification - timeinfo.tm_year = 1970 + timeinfo.tm_year; + // Positive Case - Anti-pattern 1: [year ±n, month, day] + timeinfo.tm_year = x + timeinfo.tm_year; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] st.wYear = x; // BUG - UncheckedLeapYearAfterYearModification - st.wYear = x + st.wYear; - // BUG - UncheckedLeapYearAfterYearModification - st.wYear = (1986 + st.wYear) - 1; + // Positive Case - Anti-pattern 1: [year ±n, month, day] + st.wYear = x + st.wYear; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] } -// False positive -inline void -IncrementMonth(LPSYSTEMTIME pst) +/** + * Positive AntiPattern 1 NOTE: historically considered positive but mktime checks year validity, needs re-assessment + * Year field is modified but via an intermediary variable. +*/ +bool tp_intermediaryVar(struct timespec now, struct logtime ×tamp_remote) { - if (pst->wMonth < 12) + struct tm tm_parsed; + bool timestamp_found = false; + + struct tm tm_now; + time_t t_now; + int year; + + timestamp_found = true; + + /* + * As the timestamp does not contain the year + * number, daylight saving time information, nor + * a time zone, attempt to infer it. Due to + * clock skews, the timestamp may even be part + * of the next year. Use the last year for which + * the timestamp is at most one week in the + * future. + * + * This loop can only run for at most three + * iterations before terminating. + */ + t_now = now.tv_sec; + localtime_r(&t_now, &tm_now); + + timestamp_remote.tm = tm_parsed; + timestamp_remote.tm.tm_isdst = -1; + timestamp_remote.usec = now.tv_nsec * 0.001; + for (year = tm_now.tm_year + 1;; --year) { - pst->wMonth++; + // assert(year >= tm_now.tm_year - 1); + timestamp_remote.tm.tm_year = year; + if (mktime(×tamp_remote.tm) < t_now + 7 * 24 * 60 * 60) + break; } - else +} + + + // False positive + inline void + IncrementMonth(LPSYSTEMTIME pst) + { + if (pst->wMonth < 12) + { + pst->wMonth++; + } + else + { + pst->wMonth = 1; + pst->wYear++; + } + } + + ///////////////////////////////////////////////////////// + + void mkDateTest(int year) { - pst->wMonth = 1; - pst->wYear++; + struct tm t; + + t.tm_sec = 0; + t.tm_min = 0; + t.tm_hour = 0; + t.tm_mday = 1; // day of the month - [1, 31] + t.tm_mon = 0; // months since January - [0, 11] + if (year >= 1900) + { + // 4-digit year + t.tm_year = year - 1900; // GOOD + } + else if ((year >= 0) && (year < 100)) + { + // 2-digit year assumed in the range 2000 - 2099 + t.tm_year = year + 100; // GOOD [FALSE POSITIVE] + } + else + { + // fail + } + // ... } -} -///////////////////////////////////////////////////////// + /** + * Negative Case - Anti-pattern 1a: [a.year, b.month, b.day] + * False positive: No modification of SYSTEMTIME struct. + */ + void unmodified1() + { + SYSTEMTIME st; + FILETIME ft; + WORD w; -void mkDateTest(int year) -{ - struct tm t; + GetSystemTime(&st); + + w = st.wYear; + + SystemTimeToFileTime(&st, &ft); // GOOD - no modification + } + + /** + * Negative Case - Anti-pattern 1a: [a.year, b.month, b.day] + * False positive: No modification of SYSTEMTIME struct. + */ + void unmodified2() + { + SYSTEMTIME st; + FILETIME ft; + WORD *w_ptr; + + GetSystemTime(&st); + + w_ptr = &(st.wYear); + + SystemTimeToFileTime(&st, &ft); // GOOD - no modification + } - t.tm_sec = 0; - t.tm_min = 0; - t.tm_hour = 0; - t.tm_mday = 1; // day of the month - [1, 31] - t.tm_mon = 0; // months since January - [0, 11] - if (year >= 1900) + /** + * Positive Case - Anti-pattern 1: [year ±n, month, day] + * Modification of SYSTEMTIME struct adding to year but no leap year guard is conducted. + */ + void modified3() { - // 4-digit year - t.tm_year = year - 1900; // GOOD - } else if ((year >= 0) && (year < 100)) { - // 2-digit year assumed in the range 2000 - 2099 - t.tm_year = year + 100; // GOOD [FALSE POSITIVE] - } else { - // fail + SYSTEMTIME st; + FILETIME ft; + WORD *w_ptr; + + GetSystemTime(&st); + + // BUG - UncheckedLeapYearAfterYearModification + st.wYear = st.wYear + 1; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] + + SystemTimeToFileTime(&st, &ft); + } + + /** + * Positive Case - Anti-pattern 1: [year ±n, month, day] + * Modification of SYSTEMTIME struct adding to year but no leap year guard is conducted. + */ + void modified4() + { + SYSTEMTIME st; + FILETIME ft; + WORD *w_ptr; + + GetSystemTime(&st); + + // BUG - UncheckedLeapYearAfterYearModification + st.wYear++; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] + + SystemTimeToFileTime(&st, &ft); + } + + /** + * Negative Case - Anti-pattern 1: [year ±n, month, day] + * Modification of SYSTEMTIME struct adding to year but value passed to a + * conversion function that can be checked for success, and the result is checked. + */ + void modified5() + { + SYSTEMTIME st; + FILETIME ft; + WORD *w_ptr; + + GetSystemTime(&st); + + st.wYear++; + + // Presumed safe usage, as if the conversion is incorrect, a user can handle the error. + // NOTE: it doesn't mean the user actually does the correct conversion and it it also + // doesn't mean it will error our in all cases that may be invalid. + // For example, if a leap year and the date is 28, we may want 29 if the time is meant + // to capture the end of the month, but 28 is still valid and will not error out. + if (SystemTimeToFileTime(&st, &ft)) + { + ///... + } + } + + struct tm ltime(void) + { + SYSTEMTIME st; + struct tm tm; + bool isLeapYear; + + GetLocalTime(&st); + tm.tm_sec=st.wSecond; + tm.tm_min=st.wMinute; + tm.tm_hour=st.wHour; + tm.tm_mday=st.wDay; + tm.tm_mon=st.wMonth-1; + tm.tm_year=(st.wYear>=1900?st.wYear-1900:0); + + // Check for leap year, and adjust the date accordingly + isLeapYear = tm.tm_year % 4 == 0 && (tm.tm_year % 100 != 0 || tm.tm_year % 400 == 0); + tm.tm_mday = tm.tm_mon == 2 && tm.tm_mday == 29 && !isLeapYear ? 28 : tm.tm_mday; + return tm; } - // ... + +/** +* Negative Case - Anti-pattern 1: [year ±n, month, day] +* Modification of SYSTEMTIME struct by copying from another struct, but no arithmetic is performed. +*/ +bool +FMAPITimeToSysTimeW(LPCWSTR wszTime, SYSTEMTIME *psystime) +{ + // if (!wszTime || SafeIsBadReadPtr(wszTime, 1) || lstrlenW(wszTime) != cchMAPITime) + // return false; + // AssertTag(!SafeIsBadWritePtr(psystime, sizeof(SYSTEMTIME)), 0x0004289a /* tag_abc80 */); + // memset(psystime, 0, sizeof(SYSTEMTIME)); + + psystime->wYear = (WORD)_wtoi(wszTime); + psystime->wMonth = (WORD)_wtoi(wszTime+5); + psystime->wDay = (WORD)_wtoi(wszTime+8); + psystime->wHour = (WORD)_wtoi(wszTime+11); + psystime->wMinute = (WORD)_wtoi(wszTime+14); + return true; } -void unmodified1() +/** +* Negative Case - Anti-pattern 1: [year ±n, month, day] +* Modification of SYSTEMTIME struct by copying from another struct, but no arithmetic is performed. +*/ +bool +ATime_HrGetSysTime(SYSTEMTIME *pst) { + // if (!FValidSysTime()) + // { + // TrapSzTag("ATime cannot be converted to SYSTEMTIME", 0x1e14f5c3 /* tag_4fpxd */); + // CORgTag(E_FAIL, 0x6c373230 /* tag_l720 */); + // } + + // pst->wYear = static_cast(m_lYear); + // pst->wMonth = static_cast(m_lMonth); + // //pst->wDayOfWeek = ???; + // pst->wDay = static_cast(m_lDay); + // pst->wHour = static_cast(m_lHour); + // pst->wMinute = static_cast(m_lMinute); + // pst->wSecond = static_cast(m_lSecond); + // pst->wMilliseconds = 0; +} + +/** +* Negative Case - Anti-pattern 1: [year ±n, month, day] +* Modification of SYSTEMTIME struct by copying from another struct, but no arithmetic is performed. +*/ +void fp_daymonth_guard(){ SYSTEMTIME st; FILETIME ft; - WORD w; + GetSystemTime(&st); + // FALSE POSITIVE: year is incremented but month is checked and day corrected + // in a ternary operation. It may be possible to fix this with a more sophisticated + // data flow analysis. + st.wYear++; // $ SPURIOUS: Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] + + st.wDay = st.wMonth == 2 && st.wDay == 29 ? 28 : st.wDay; + + SystemTimeToFileTime(&st, &ft); +} + +void increment_arg(WORD &x){ + x++; // $ Source +} + +void increment_arg_by_pointer(WORD *x){ + (*x)++; // $ Source +} + +void fn_year_set_through_out_arg(){ + SYSTEMTIME st; GetSystemTime(&st); + // BAD, year incremented without check + increment_arg(st.wYear); // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] + + // GetSystemTime(&st); + // Bad, year incremented without check + increment_arg_by_pointer(&st.wYear); // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] +} + - w = st.wYear; +/* TODO: don't alert on simple copies from another struct where all three {year,month,day} are copied +void +GetEpochTime(struct pg_tm *tm) +{ + struct pg_tm *t0; + pg_time_t epoch = 0; + + t0 = pg_gmtime(&epoch); + + tm->tm_year = t0->tm_year; + tm->tm_mon = t0->tm_mon; + tm->tm_mday = t0->tm_mday; + tm->tm_hour = t0->tm_hour; + tm->tm_min = t0->tm_min; + tm->tm_sec = t0->tm_sec; + + tm->tm_year += 1900; + tm->tm_mon++; +} */ + +void +fp_guarded_by_month(struct pg_tm *tm){ + int woy = 52; + int MONTHS_PER_YEAR = 12; + /* + * If it is week 52/53 and the month is January, then the + * week must belong to the previous year. Also, some + * December dates belong to the next year. + */ + if (woy >= 52 && tm->tm_mon == 1) + --tm->tm_year; // Negative Test Case + if (woy <= 1 && tm->tm_mon == MONTHS_PER_YEAR) + ++tm->tm_year; // Negative Test Case +} - SystemTimeToFileTime(&st, &ft); // GOOD - no modification +typedef unsigned short CSHORT; + +typedef struct _TIME_FIELDS { + CSHORT Year; + CSHORT Month; + CSHORT Day; + CSHORT Hour; + CSHORT Minute; + CSHORT Second; + CSHORT Milliseconds; + CSHORT Weekday; +} TIME_FIELDS, *PTIME_FIELDS; + +void +tp_ptime(PTIME_FIELDS ptm){ + ptm->Year = ptm->Year - 1; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] } -void unmodified2() + +bool isLeapYearRaw(WORD year) { - SYSTEMTIME st; - FILETIME ft; - WORD *w_ptr; + return year % 4 == 0 && (year % 100 != 0 || year % 400 == 0); +} - GetSystemTime(&st); +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; + + year += offset; // $ Source - w_ptr = &(st.wYear); + tmp.tm_year = year; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] - SystemTimeToFileTime(&st, &ft); // GOOD - no modification } -void modified3() + +bool isNotLeapYear(struct tm tm) { - SYSTEMTIME st; + return !(tm.tm_year % 4 == 0 && (tm.tm_year % 100 != 0 || tm.tm_year % 400 == 0)); +} + +bool isNotLeapYear2(struct tm tm) +{ + return (tm.tm_year % 4 != 0 || (tm.tm_year % 100 == 0 && tm.tm_year % 400 != 0)); +} + + +void inverted_leap_year_check(WORD year, WORD offset, WORD day){ + struct tm tmp; + + tmp.tm_year = year + offset; + + if (isNotLeapYear(tmp)){ + day = 28; + } + + tmp.tm_year = year + offset; + + if(isNotLeapYear2(tmp)){ + day = 28; + } + + + tmp.tm_year = year + offset; + bool isNotLeapYear = (tmp.tm_year % 4 != 0 || (tmp.tm_year % 100 == 0 && tmp.tm_year % 400 != 0)); + + if(isNotLeapYear){ + day = 28; + } + + tmp.tm_year = year + offset; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] +} + + +void simplified_leap_year_check1(WORD year, WORD offset){ + struct tm tmp; + + tmp.tm_year = year + offset; // OK + + bool isLeap = (!((tmp.tm_year + 1900) % 4)) && ((tmp.tm_year + 1900) % 100 || !((tmp.tm_year + 1900) % 400)); + if(isLeap){ + // do something + } + + // Modified after check, could be dangerous + tmp.tm_year = year + offset; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] +} + +void simplified_leap_year_check2(WORD year, WORD offset){ + struct tm tmp; + + tmp.tm_year = year + offset; // OK + + bool isNotLeap = ((tmp.tm_year + 1900) % 4) || (!((tmp.tm_year + 1900) % 100) && ((tmp.tm_year + 1900) % 400)); + if(isNotLeap){ + // do something + } + + // Modified after check, could be dangerous + tmp.tm_year = year + offset; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] +} + +void simplified_leap_year_check3(WORD year, WORD offset){ + SYSTEMTIME tmp; + + tmp.wYear = year + offset; // OK + + bool isLeap = (!(tmp.wYear % 4)) && (tmp.wYear % 100 || !(tmp.wYear% 400)); + if(isLeap){ + // do something + } + + // Modified after check, could be dangerous + tmp.wYear = year + offset; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] +} + +void simplified_leap_year_check4(WORD year, WORD offset){ + SYSTEMTIME tmp; + + tmp.wYear = year + offset; // OK + + bool isNotLeap = (tmp.wYear % 4) || (!(tmp.wYear % 100) && (tmp.wYear % 400)); + if(isNotLeap){ + // do something + } + + // Modified after check, could be dangerous + tmp.wYear = year + offset; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] +} + +void bad_simplified_leap_year_check1(WORD year, WORD offset){ + struct tm tmp; + + tmp.tm_year = year + offset; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] + + // incorrect logic, should negate the %4 result + bool isLeap = ((tmp.tm_year + 1900) % 4) && ((tmp.tm_year + 1900) % 100 || !((tmp.tm_year + 1900) % 400)); + if(isLeap){ + // do something + } +} + +void bad_simplified_leap_year_check2(WORD year, WORD offset){ + struct tm tmp; + + tmp.tm_year = year + offset; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] + + + // incorrect logic, should not negate the %4 result + bool isNotLeap = (!((tmp.tm_year + 1900) % 4)) || (!((tmp.tm_year + 1900) % 100) && ((tmp.tm_year + 1900) % 400)); + if(isNotLeap){ + // do something + } +} + +void bad_simplified_leap_year_check3(WORD year, WORD offset){ + SYSTEMTIME tmp; + + tmp.wYear = year + offset; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] + + // incorrect logic, should negate the %4 result + bool isLeap = (tmp.wYear % 4) && (tmp.wYear % 100 || !(tmp.wYear % 400)); + if(isLeap){ + // do something + } +} + +void bad_simplified_leap_year_check4(WORD year, WORD offset){ + SYSTEMTIME tmp; + + tmp.wYear = year + offset; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] + + + // incorrect logic, should not negate the %4 result + bool isNotLeap = (!(tmp.wYear % 4)) || (!(tmp.wYear % 100) && (tmp.wYear % 400)); + if(isNotLeap){ + // do something + } +} + + +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; - WORD *w_ptr; - GetSystemTime(&st); + // (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); - st.wYear = st.wYear + 1; // BAD + // Assume this check of the result is sufficient as an implicit leap year check. + bool x = (res == 0) ? true : false; +} - SystemTimeToFileTime(&st, &ft); +void set_time(WORD year, WORD month, WORD day){ + SYSTEMTIME tmp; + + tmp.wYear = year; //$ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] + tmp.wMonth = month; + tmp.wDay = day; +} + +void constant_month_on_year_modification1(WORD year, WORD offset, WORD month){ + SYSTEMTIME tmp; + + if(month++ > 12){ + tmp.wMonth = 1; + tmp.wYear = year + 1;// OK since the year is incremented with a known non-leap year month change + } + + if(month++ > 12){ + + set_time(year+1, 1, 31);// OK since the year is incremented with a known non-leap year month change + } +} + +void constant_month_on_year_modification2(WORD year, WORD offset, WORD month){ + SYSTEMTIME tmp; + + if(month++ > 12){ + tmp.wMonth = 1; + tmp.wYear = year + 1;// OK since the year is incremented with a known non-leap year month change + } + + + if(month++ > 12){ + // some hueristics to detect a false positive here rely on variable names + // which is often consistent in the wild. + // This variant uses the variable names yeartmp and monthtmp + WORD yeartmp; + WORD monthtmp; + yeartmp = year + 1; + monthtmp = 1; + set_time(yeartmp, monthtmp, 31);// OK since the year is incremented with a known non-leap year month change + } +} + +typedef struct parent_struct { + SYSTEMTIME t; +} PARENT_STRUCT; + + + +void nested_time_struct(WORD year, WORD offset){ + PARENT_STRUCT ps; + + ps.t.wYear = year + offset; // OK, checked below + + bool isLeap = isLeapYearRaw(ps.t.wYear); + + if(isLeap){ + // do something + } +} + +void intermediate_time_struct(WORD year, WORD offset){ + SYSTEMTIME tm, tm2; + FILETIME ftTime; + + tm.wYear = year + offset; + + tm2.wYear = tm.wYear; + + + while ( !SystemTimeToFileTime( &tm2, &ftTime ) ) + { + /// handle error + } + +} + +void constant_day_on_year_modification1(WORD year, WORD offset, WORD month){ + SYSTEMTIME tmp; + + if(month++ > 12){ + tmp.wDay = 1; + tmp.wYear = year + 1;// OK since the year is incremented with a known non-leap year day + } + + if(month++ > 12){ + + set_time(year+1, month, 1);// OK since the year is incremented with a known non-leap year day + } + + if(month++ > 12){ + + // BAD, year incremented, month unknown in block, and date is set to 31 + // which is dangerous. + set_time(year+1, month, 31);// $ Source + } +} + +void constant_day_on_year_modification2(WORD year, WORD month){ + SYSTEMTIME tmp; + + // FLASE POSITIVE SOURCE: + // flowing into set_time, the set time does pass a constant day + // but the source here and the source of that constant month don't align + // Current heuristics require the source of the constant day align with the + // source and/or the sink of the year modification. + // We could potentially improve this by checking the paths of both the year and day + // flows, but this may be more complex than is warranted for now. + year = year + 1; // $ SPURIOUS: Source + + if(month++ > 12){ + tmp.wDay = 1; + tmp.wYear = year;// OK since the year is incremented with a known non-leap year day + } + + if(month++ > 12){ + + set_time(year, month, 1);// OK since the year is incremented with a known non-leap year day + } + + year = year + 1; // $ Source + + if(month++ > 12){ + + // BAD, year incremented, month unknown in block, and date is set to 31 + // which is dangerous. + set_time(year, month, 31); + } +} + + +void modification_after_conversion1(tm timeinfo){ + // convert a tm year into a civil year, then modify after conversion + // This case shows a false negative where the year might be used and it is incorrectly modified, + // and never reassigned to another struct. + WORD year = timeinfo.tm_year + 1900; + + year += 1; // $ MISSING: Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] +} + +WORD get_civil_year(tm timeinfo){ + return timeinfo.tm_year + 1900; +} + +void modification_after_conversion2(tm timeinfo){ + // convert a tm year into a civil year, then modify after conversion + // This case shows a false negative where the year might be used and it is incorrectly modified, + // and never reassigned to another struct. + WORD year = get_civil_year(timeinfo); + year += 1; // $ MISSING: Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] +} + +void modification_after_conversion_saved_to_other_time_struct1(tm timeinfo){ + // convert a tm year into a civil year, then modify after conversion + // This case shows a false negative where the year might be used and it is incorrectly modified, + // and never reassigned to another struct. + WORD year = timeinfo.tm_year + 1900; + + year += 1; // $ MISSING: Source + + SYSTEMTIME s; + // FALSE NEGATIVE: missing this because the conversion happens locally before + // the year adjustment, which seems as though it is part of a conversion itself + s.wYear = year; // $ MISSING: Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] +} + + + +void modification_after_conversion_saved_to_other_time_struct2(tm timeinfo){ + // convert a tm year into a civil year, then modify after conversion + // This case shows a false negative where the year might be used and it is incorrectly modified, + // and never reassigned to another struct. + WORD year = get_civil_year(timeinfo); + + year += 1; // $ Source + + SYSTEMTIME s; + s.wYear = year; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] } -void modified4() +void modification_after_conversion_saved_to_other_time_struct3(tm timeinfo){ + // convert a tm year into a civil year, then modify after conversion + // This case shows a false negative where the year might be used and it is incorrectly modified, + // and never reassigned to another struct. + WORD year = timeinfo.tm_year + 1900; + + year = year + 1; // $ MISSING: Source + + SYSTEMTIME s; + // FALSE NEGATIVE: missing this because the conversion happens locally before + // the year adjustment, which seems as though it is part of a conversion itself + s.wYear = year; // $ MISSING: Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] +} + + +void year_saved_to_variable_then_modified1(tm timeinfo){ + // A modified year is not directly assigned to the year, rather, the year is + // saved to a variable, modified, used, but never assigned back. + WORD year = timeinfo.tm_year; + + // NOTE: should we even try to detect cases like this? + // Our current rationale is that a year in a struct is more dangerous than a year in isolation + // A year in isolation is harder to interpret + year += 1; // MISSING: $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] +} + +void modification_before_conversion1(tm timeinfo){ + timeinfo.tm_year += 1; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] + // convert a tm year into a civil year, then modify after conversion + // This case shows a false negative where the year might be used and it is incorrectly modified, + // and never reassigned to another struct. + WORD year = timeinfo.tm_year + 1900; +} + +void modification_before_conversion2(tm timeinfo){ + timeinfo.tm_year += 1; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] + // convert a tm year into a civil year, then modify after conversion + // This case shows a false negative where the year might be used and it is incorrectly modified, + // and never reassigned to another struct. + WORD year = get_civil_year(timeinfo); +} + + + +void year_saved_to_variable_then_modified_with_leap_check1(tm timeinfo){ + // A modified year is not directly assigned to the year, rather, the year is + // saved to a variable, modified, used, but never assigned back. + WORD year = timeinfo.tm_year; + + year += 1; + + // performing a check is considered good enough, even if not used correctly + bool b = (year+1900) % 4 == 0 && ((year+1900) % 100 != 0 || (year+1900) % 400 == 0); +} + + +void modification_after_conversion_with_leap_check1(tm timeinfo){ + // convert a tm year into a civil year, then modify after conversion + // This case shows a false negative where the year might be used and it is incorrectly modified, + // and never reassigned to another struct. + WORD year = timeinfo.tm_year + 1900; + + year += 1; + + // performing a check is considered good enough, even if not used correctly + bool b = year % 4 == 0 && (year % 100 != 0 || year % 400 == 0); +} + +void modification_after_conversion_with_leap_check2(tm timeinfo){ + // convert a tm year into a civil year, then modify after conversion + // This case shows a false negative where the year might be used and it is incorrectly modified, + // and never reassigned to another struct. + WORD year = get_civil_year(timeinfo); + + year += 1; + + // performing a check is considered good enough, even if not used correctly + bool b = year % 4 == 0 && (year % 100 != 0 || year % 400 == 0); +} + +void modification_before_conversion_with_leap_check1(tm timeinfo){ + timeinfo.tm_year += 1; + // convert a tm year into a civil year, then modify after conversion + // This case shows a false negative where the year might be used and it is incorrectly modified, + // and never reassigned to another struct. + WORD year = timeinfo.tm_year + 1900; + + // performing a check is considered good enough, even if not used correctly + bool b = year % 4 == 0 && (year % 100 != 0 || year % 400 == 0); +} + +void modification_before_conversion_with_leap_check2(tm timeinfo){ + timeinfo.tm_year += 1; + // convert a tm year into a civil year, then modify after conversion + // This case shows a false negative where the year might be used and it is incorrectly modified, + // and never reassigned to another struct. + WORD year = get_civil_year(timeinfo); + + // performing a check is considered good enough, even if not used correctly + bool b = (year) % 4 == 0 && ((year) % 100 != 0 || (year) % 400 == 0); +} + +void odd_leap_year_check1(tm timeinfo){ + timeinfo.tm_year += 1; + + + // Using an odd sytle of checking divisible by 4 presumably as an optimization trick + if(((timeinfo.tm_year+1900) & 3) == 0 && ((timeinfo.tm_year+1900) % 100 != 0 || (timeinfo.tm_year+1900) % 400 == 0)) + { + // do something + } +} + +void odd_leap_year_check2(tm timeinfo){ + timeinfo.tm_year += 1; // $ SPURIOUS: Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] + + // Using an odd sytle of checking divisible by 4 presumably as an optimization trick + // but also check unrelated conditions on the year as an optimization to rule out irrelevant years + // for gregorian leap years + if(timeinfo.tm_mon == 2 && ((timeinfo.tm_year+1900) & 3) == 0 && ((timeinfo.tm_year+1900) <= 1582 || (timeinfo.tm_year+1900) % 100 != 0 || (timeinfo.tm_year+1900) % 400 == 0)) + { + // do something + } +} + +void odd_leap_year_check3(tm timeinfo){ + timeinfo.tm_year += 1; // $ SPURIOUS: Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] + + // Using an odd sytle of checking divisible by 4 presumably as an optimization trick + // but also check unrelated conditions on the year as an optimization to rule out irrelevant years + // for gregorian leap years + if(timeinfo.tm_mon == 2 && ((timeinfo.tm_year+1900) % 4) == 0 && ((timeinfo.tm_year+1900) <= 1582 || (timeinfo.tm_year+1900) % 100 != 0 || (timeinfo.tm_year+1900) % 400 == 0)) + { + // do something + } +} + +void odd_leap_year_check4(tm timeinfo){ + timeinfo.tm_year += 1; + WORD year = timeinfo.tm_year + 1900; + + if( (year % 4 == 0) && (year % 100 > 0 || (year % 400 == 0))) + { + // do something + } +} + +void odd_leap_year_check5(tm timeinfo){ + timeinfo.tm_year += 1; + WORD year = timeinfo.tm_year + 1900; + + if( (year % 4 > 0) || (year % 100 == 0 && (year % 400 > 0))) + { + // do something + } +} + + +void date_adjusted_through_mkgmtime(tm timeinfo){ + timeinfo.tm_year += 1; // $ SPURIOUS: Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] + + // Using an odd sytle of checking divisible by 4 presumably as an optimization trick + // but also check unrelated conditions on the year as an optimization to rule out irrelevant years + // for gregorian leap years + if(timeinfo.tm_mon == 2 && ((timeinfo.tm_year+1900) % 4) == 0 && ((timeinfo.tm_year+1900) <= 1582 || (timeinfo.tm_year+1900) % 100 != 0 || (timeinfo.tm_year+1900) % 400 == 0)) + { + // do something + } +} + +bool data_killer(WORD *d){ + (*d) = 1; + return true; +} + +void interproc_data_killer1(tm timeinfo, WORD delta){ + WORD year = delta + 1; + + if(data_killer(&year)){ + timeinfo.tm_year = year; + } +} + + +void leap_year_check_after_normalization(tm timeinfo, WORD delta){ + WORD year = delta + 1; + + if(data_killer(&year)){ + timeinfo.tm_year = year; + } +} + + +void leap_year_check_call_on_conversion1(tm timeinfo){ + timeinfo.tm_year += 1; + isLeapYearRaw(timeinfo.tm_year + 1900); +} + +void leap_year_check_call_on_conversion2(tm timeinfo){ + timeinfo.tm_year += 1; + WORD year = get_civil_year(timeinfo); + isLeapYearRaw(year); +} + +WORD getDaysInMonth(WORD year, WORD month){ + // simplified + if(month == 2){ + return isLeapYearRaw(year) ? 29 : 28; + } + // else assume logic for every other month, + // returning 30 for simplicity + return 30; +} + +WORD get_civil_year_raw(WORD year){ + return year + 1900; +} + +void leap_year_check_call_on_conversion3(tm timeinfo, WORD year, WORD month, WORD delta){ + year += delta; + WORD days = getDaysInMonth(get_civil_year_raw(year), month); + timeinfo.tm_year = year; +} + +void assumed_maketime_conversion1(tm timeinfo) { - SYSTEMTIME st; - FILETIME ft; - WORD *w_ptr; + //the docs of mktime suggest feb29 is handled, and conversion will occur automatically + //no check required. + timeinfo.tm_year += 1; - GetSystemTime(&st); + mktime(&timeinfo); +} - st.wYear++; // BAD - st.wYear++; // BAD - st.wYear++; // BAD - SystemTimeToFileTime(&st, &ft); +void bad_leap_year_check_logic1(tm timeinfo){ + timeinfo.tm_year += 1; // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] + + WORD year = get_civil_year(timeinfo); + + // expected logic: + //(year % 4) && ((year % 100) || !(year % 400 ))) + WORD days = (!(year % 4) && (!(year % 100) || (year % 400))) ? 366 : 365; } + From 95d4a541bcdec7096807c03c617370448a3f4c92 Mon Sep 17 00:00:00 2001 From: "REDMOND\\brodes" Date: Fri, 6 Feb 2026 15:42:43 -0500 Subject: [PATCH 2/8] C++: Refactor leap year logic for UncheckedLeapYearAfterYearModification. Includes new logic for detecting leap year checks, new forms of leap year checks detected, and various heuristics to remove false postives. Move TimeConversionFunction into LeapYear.qll and refactored to separate conversion functions that are expected to be checked for failure from those that auto correct leap year dates if feb 29 is provided on a non-leap year. Increas the set of known TimeConversionFunctions. --- cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll | 32 + .../UncheckedLeapYearAfterYearModification.ql | 853 +++++++++++++++++- .../UncheckedReturnValueForTimeFunctions.ql | 15 - 3 files changed, 850 insertions(+), 50 deletions(-) diff --git a/cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll b/cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll index 2b68730fa58d..77c23d97b020 100644 --- a/cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll +++ b/cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll @@ -308,3 +308,35 @@ private module PossibleYearArithmeticOperationCheckConfig implements DataFlow::C module PossibleYearArithmeticOperationCheckFlow = TaintTracking::Global; + +/** + * This list of APIs should check for the return value to detect problems during the conversion. + */ +class TimeConversionFunction extends Function { + boolean autoLeapYearCorrecting; + + TimeConversionFunction() { + autoLeapYearCorrecting = false and + ( + this.getName() = + [ + "FileTimeToSystemTime", "SystemTimeToFileTime", "SystemTimeToTzSpecificLocalTime", + "SystemTimeToTzSpecificLocalTimeEx", "TzSpecificLocalTimeToSystemTime", + "TzSpecificLocalTimeToSystemTimeEx", "RtlLocalTimeToSystemTime", + "RtlTimeToSecondsSince1970", "_mkgmtime", "SetSystemTime", "VarUdateFromDate", "from_tm" + ] + or + // Matches all forms of GetDateFormat, e.g. GetDateFormatA/W/Ex + this.getName().matches("GetDateFormat%") + ) + or + autoLeapYearCorrecting = true and + this.getName() = + ["mktime", "_mktime32", "_mktime64", "SystemTimeToVariantTime", "VariantTimeToSystemTime"] + } + + /** + * Holds if the function is expected to auto convert a bad leap year date. + */ + predicate isAutoLeapYearCorrecting() { autoLeapYearCorrecting = true } +} diff --git a/cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql b/cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql index 03570b3611cd..8be4bbfbfe41 100644 --- a/cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql +++ b/cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql @@ -1,7 +1,7 @@ /** * @name Year field changed using an arithmetic operation without checking for leap year * @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. - * @kind problem + * @kind path-problem * @problem.severity warning * @id cpp/leap-year/unchecked-after-arithmetic-year-modification * @precision medium @@ -11,49 +11,832 @@ import cpp import LeapYear +import semmle.code.cpp.controlflow.IRGuards +import semmle.code.cpp.ir.IR +import semmle.code.cpp.commons.DateTime -from Variable var, LeapYearFieldAccess yfa -where - exists(VariableAccess va | - yfa.getQualifier() = va and - var.getAnAccess() = va and - // The year is modified with an arithmetic operation. Avoid values that are likely false positives - yfa.isModifiedByArithmeticOperationNotForNormalization() and - // Avoid false positives - not ( - // If there is a local check for leap year after the modification - exists(LeapYearFieldAccess yfacheck | - yfacheck.getQualifier() = var.getAnAccess() and - yfacheck.isUsedInCorrectLeapYearCheck() and - yfacheck.getBasicBlock() = yfa.getBasicBlock().getASuccessor*() +/** + * 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 + // helper utility for date conversions in qtbase + this.getName() = "adjacentDay" + or + // Windows API function that does timezone conversions + this.getName().matches("%SystemTimeToTzSpecificLocalTime%") + or + // Windows APIs that do time conversions + this.getName().matches("%localtime%\\_s%") + or + // Windows APIs that do time conversions + this.getName().matches("%SpecificLocalTimeToSystemTime%") + or + // postgres function for diffing timestamps, date for leap year + // is not applicable. + this.getName().toLowerCase().matches("%timestamp%age%") + or + // Reading byte streams often involves operations of some base, but that's + // not a real source of leap year issues. + this.getName().toLowerCase().matches("%read%bytes%") + or + // A postgres function for local time conversions + // conversion operations (from one time structure to another) are generally ignorable + this.getName() = "localsub" + or + // Indication of a calendar not applicable to + // gregorian leap year, e.g., Hijri, Persian, Hebrew + this.getName().toLowerCase().matches("%hijri%") + or + this.getFile().getBaseName().toLowerCase().matches("%hijri%") + or + this.getName().toLowerCase().matches("%persian%") + or + this.getFile().getBaseName().toLowerCase().matches("%persian%") + or + this.getName().toLowerCase().matches("%hebrew%") + or + this.getFile().getBaseName().toLowerCase().matches("%hebrew%") + or + // misc. from string/char converters heuristic + 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%") + } +} + +/** + * The set of expressions which are ignorable; either because they seem to not be part of a year mutation, + * or because they seem to be a conversion pattern of mapping date scalars. + */ +abstract class IgnorableOperation extends Expr { } + +class IgnorableExprRem extends IgnorableOperation instanceof RemExpr { } + +/** + * Anything involving an operation with 10, 100, 1000, 10000 is often a sign of conversion + * or atoi. + */ +class IgnorableExpr10MulipleComponent extends IgnorableOperation { + IgnorableExpr10MulipleComponent() { + this.(Operation).getAnOperand().getValue().toInt() in [10, 100, 1000, 10000] + or + exists(AssignOperation a | a.getRValue() = this | + a.getRValue().getValue().toInt() in [10, 100, 1000, 10000] + ) + } +} + +/** + * Anything involving a sub expression with char literal 48, ignore as a likely string conversion + * e.g., X - '0' + */ +class IgnorableExpr48Mapping extends IgnorableOperation { + IgnorableExpr48Mapping() { + this.(SubExpr).getRightOperand().getValue().toInt() = 48 + or + exists(AssignSubExpr e | e.getRValue() = this | e.getRValue().getValue().toInt() = 48) + } +} + +/** + * A binary or arithemtic operation whereby one of the components is textual or a string. + */ +class IgnorableCharLiteralArithmetic extends IgnorableOperation { + IgnorableCharLiteralArithmetic() { + this.(BinaryArithmeticOperation).getAnOperand() instanceof TextLiteral + or + this instanceof TextLiteral and + any(AssignArithmeticOperation arith).getRValue() = this + } +} + +/** + * Constants often used in date conversions (from one date data type to another) + * Numerous examples exist, like 1900 or 2000 that convert years from one + * representation to another. + * Also '0' is sometimes observed as an atoi style conversion. + */ +bindingset[c] +predicate isLikelyConversionConstant(int c) { + exists(int i | i = c.abs() | + i = + [ + 146097, // days in 400-year Gregorian cycle + 36524, // days in 100-year Gregorian subcycle + 1461, // days in 4-year cycle (incl. 1 leap) + 32044, // Fliegel–van Flandern JDN epoch shift + 1721425, // JDN of 0001‑01‑01 (Gregorian) + 1721119, // alt epoch offset + 2400000, // MJD → JDN conversion + 2400001, // alt MJD → JDN conversion + 2141, // fixed‑point month/day extraction + 65536, // observed in some conversions + 7834, // observed in some conversions + 256, // observed in some conversions + 292275056, // qdatetime.h Qt Core year range first year constant + 292278994, // qdatetime.h Qt Core year range last year constant + 1601, // Windows FILETIME epoch start year + 1970, // Unix epoch start year + 70, // Unix epoch start year short form + 1899, // Observed in uses with 1900 to address off by one scenarios + 1900, // Used when converting a 2 digit year + 2000, // Used when converting a 2 digit year + 1400, // Hijri base year, used when converting a 2 digit year + 1980, // FAT filesystem epoch start year + 227013, // constant observed for Hirji year conversion, and Hirji years are not applicable for gregorian leap year + 10631, // constant observed for Hirji year conversion, and Hirji years are not applicable for gregorian leap year + 0 + ] + ) +} + +/** + * Some constants indicate conversion that are ignorable, e.g., + * julian to gregorian conversion or conversions from linux time structs + * that start at 1900, etc. + */ +class IgnorableConstantArithmetic extends IgnorableOperation { + IgnorableConstantArithmetic() { + exists(int i | isLikelyConversionConstant(i) | + this.(Operation).getAnOperand().getValue().toInt() = i + or + exists(AssignArithmeticOperation a | this = a.getRValue() | + a.getRValue().getValue().toInt() = i ) + ) + } +} + +// If a unary minus assume it is some sort of conversion +class IgnorableUnaryMinus extends IgnorableOperation { + IgnorableUnaryMinus() { + this instanceof UnaryMinusExpr + or + this.(Operation).getAnOperand() instanceof UnaryMinusExpr + } +} + +/** + * An argument to a function is ignorable if the function that is called is an ignored function + */ +class OperationAsArgToIgnorableFunction extends IgnorableOperation { + OperationAsArgToIgnorableFunction() { + exists(Call c | + c.getAnArgument().getAChild*() = this and + c.getTarget() instanceof IgnorableFunction + ) + } +} + +/** + * Literal OP literal means the result is constant/known + * and the operation is basically ignorable (it's not a real operation but + * probably one visual simplicity what it means). + */ +class ConstantBinaryArithmeticOperation extends IgnorableOperation, BinaryArithmeticOperation { + ConstantBinaryArithmeticOperation() { + this.getLeftOperand() instanceof Literal and + this.getRightOperand() instanceof Literal + } +} + +class IgnorableBinaryBitwiseOperation extends IgnorableOperation instanceof BinaryBitwiseOperation { +} + +class IgnorableUnaryBitwiseOperation extends IgnorableOperation instanceof UnaryBitwiseOperation { } + +class IgnorableAssignmentBitwiseOperation extends IgnorableOperation instanceof AssignBitwiseOperation +{ } + +/** + * Any arithmetic operation where one of the operands is a pointer or char type, ignore it + */ +class IgnorablePointerOrCharArithmetic extends IgnorableOperation { + IgnorablePointerOrCharArithmetic() { + this instanceof BinaryArithmeticOperation and + ( + this.(BinaryArithmeticOperation).getAnOperand().getUnspecifiedType() instanceof PointerType + or + this.(BinaryArithmeticOperation).getAnOperand().getUnspecifiedType() instanceof CharType + or + // Operations on calls to functions that accept char or char* + this.(BinaryArithmeticOperation) + .getAnOperand() + .(Call) + .getAnArgument() + .getUnspecifiedType() + .stripType() instanceof CharType + or + // Operations on calls to functions named like "strlen", "wcslen", etc + // NOTE: workaround for cases where the wchar_t type is not a char, but an unsigned short + // unclear if there is a best way to filter cases like these out based on type info. + this.(BinaryArithmeticOperation).getAnOperand().(Call).getTarget().getName().matches("%len%") + ) + or + exists(AssignArithmeticOperation a | a.getRValue() = this | + a.getAnOperand().getUnspecifiedType() instanceof PointerType + or + a.getAnOperand().getUnspecifiedType() instanceof CharType + or + // Operations on calls to functions that accept char or char* + a.getAnOperand().(Call).getAnArgument().getUnspecifiedType().stripType() instanceof CharType + or + // Operations on calls to functions named like "strlen", "wcslen", etc + this.(BinaryArithmeticOperation).getAnOperand().(Call).getTarget().getName().matches("%len%") + ) + } +} + +/** + * An expression that is a candidate source for an dataflow configuration for an Operation that could flow to a Year field. + */ +predicate isOperationSourceCandidate(Expr e) { + not e instanceof IgnorableOperation and + exists(Function f | + f = e.getEnclosingFunction() and + not f instanceof IgnorableFunction + ) and + ( + e instanceof SubExpr + or + e instanceof AddExpr + or + e instanceof CrementOperation + or + e instanceof AssignSubExpr + or + e instanceof AssignAddExpr + ) +} + +/** + * A dataflow that tracks an ignorable operation (eg. bitwise op) to a operation source, so we may disqualify it. + */ +module IgnorableOperationToOperationSourceCandidateConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node n) { n.asExpr() instanceof IgnorableOperation } + + predicate isSink(DataFlow::Node n) { isOperationSourceCandidate(n.asExpr()) } + + // looking for sources and sinks in the same function + DataFlow::FlowFeature getAFeature() { + result instanceof DataFlow::FeatureEqualSourceSinkCallContext + } +} + +module IgnorableOperationToOperationSourceCandidateFlow = + TaintTracking::Global; + +/** + * The set of all expressions which is a candidate expression and also does not flow from to to some ignorable expression (eg. bitwise op) + * ``` + * a = something <<< 2; + * myDate.year = a + 1; // invalid + * ... + * a = someDate.year + 1; + * myDate.year = a; // valid + * ``` + */ +class OperationSource extends Expr { + OperationSource() { + isOperationSourceCandidate(this) and + // If the candidate came from an ignorable operation, ignore the candidate + // NOTE: we cannot easily flow the candidate to an ignorable operation as that can + // be tricky in practice, e.g., a mod operation on a year would be part of a leap year check + // but a mod operation ending in a year is more indicative of something to ignore (a conversion) + not exists(IgnorableOperationToOperationSourceCandidateFlow::PathNode sink | + sink.getNode().asExpr() = this and + sink.isSink() + ) + } +} + +class YearFieldAssignmentNode extends DataFlow::Node { + YearFieldAccess access; + + YearFieldAssignmentNode() { + exists(Function f | + f = this.getEnclosingCallable().getUnderlyingCallable() and not f instanceof IgnorableFunction + ) and + ( + this.asDefinition().(Assignment).getLValue() = access + or + this.asDefinition().(CrementOperation).getOperand() = access + or + exists(Call c | c.getAnArgument() = access and this.asDefiningArgument() = access) or - // If there is a data flow from the variable that was modified to a function that seems to check for leap year - exists(VariableAccess source, ChecksForLeapYearFunctionCall fc | - source = var.getAnAccess() and - LeapYearCheckFlow::flow(DataFlow::exprNode(source), DataFlow::exprNode(fc.getAnArgument())) + exists(Call c, AddressOfExpr aoe | + c.getAnArgument() = aoe and + aoe.getOperand() = access and + this.asDefiningArgument() = aoe ) + ) + } + + YearFieldAccess getYearFieldAccess() { result = access } +} + +/** + * A DataFlow configuration for identifying flows from some non trivial access or literal + * to the Year field of a date object. + */ +module OperationToYearAssignmentConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node n) { n.asExpr() instanceof OperationSource } + + predicate isSink(DataFlow::Node n) { + n instanceof YearFieldAssignmentNode and + not isYearModifiedWithCheck(n) and + not isControlledByMonthEqualityCheckNonFebruary(n.asExpr()) + } + + predicate isBarrier(DataFlow::Node n) { + exists(ArrayExpr arr | arr.getArrayOffset() = n.asExpr()) + or + n.getType().getUnspecifiedType() instanceof PointerType + or + n.getType().getUnspecifiedType() instanceof CharType + or + // 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 + // Flowing into variables that indicate likely non-gregorian years are barriers + // e.g., names similar to hijri, persian, lunar, chinese, hebrew, etc. + exists(Variable v | + v.getName() + .toLowerCase() + .matches(["%hijri%", "%persian%", "%lunar%", "%chinese%", "%hebrew%"]) and + v.getAnAccess() = [n.asIndirectExpr(), n.asExpr()] + ) + or + isLeapYearCheckSink(n) + or + // this is a bit of a hack to address cases where a year is normalized and checked, but the + // normalized year is never itself assigned to the final year struct + // isLeapYear(getCivilYear(year)) + // struct.year = year + // This is assuming a user would have done this all on one line though. + // setting a variable for the conversion and passing that separately would be more difficult to track + // considering this approach good enough for current observed false positives + exists(Call c, Expr arg | + isLeapYearCheckCall(c, arg) and arg.getAChild*() = [n.asExpr(), n.asIndirectExpr()] + ) + or + // If as the flow progresses, the value holding a dangerous operation result + // is apparently being passed by address to some function, it is more than likely + // intended to be modified, and therefore, the definition is killed. + exists(Call c | c.getAnArgument().(AddressOfExpr).getAnOperand() = n.asIndirectExpr()) + } + + /** Block flow out of an operation source to get the "closest" operation to the sink */ + predicate isBarrierIn(DataFlow::Node n) { isSource(n) } + + predicate isBarrierOut(DataFlow::Node n) { isSink(n) } +} + +module OperationToYearAssignmentFlow = TaintTracking::Global; + +predicate isLeapYearCheckSink(DataFlow::Node sink) { + exists(LeapYearGuardCondition lgc | + lgc.checkedYearAccess() = [sink.asExpr(), sink.asIndirectExpr()] + ) + or + isLeapYearCheckCall(_, [sink.asExpr(), sink.asIndirectExpr()]) +} + +/** + * A flow configuration from a Year field access to some Leap year check or guard + */ +module YearAssignmentToLeapYearCheckConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof YearFieldAssignmentNode } + + predicate isSink(DataFlow::Node sink) { isLeapYearCheckSink(sink) } + + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + // flow from a YearFieldAccess to the qualifier + node2.asExpr() = node1.asExpr().(YearFieldAccess).getQualifier*() + or + // Pass through any intermediate struct + exists(Assignment a, DataFlow::PostUpdateNode pun | + a.getLValue().(YearFieldAccess).getQualifier*() = pun.getPreUpdateNode().asExpr() and + a.getRValue() = node1.asExpr() and + node2.asExpr() = a.getLValue().(YearFieldAccess).getQualifier*() + ) + or + // flow from a year access qualifier to a year field + exists(YearFieldAccess yfa | node2.asExpr() = yfa and node1.asExpr() = yfa.getQualifier()) + or + // in cases of x.year = x and the x is checked, but the year x.year isn't directly + // flow from a year assignment node to an RHS if it is an assignment + exists(YearFieldAssignmentNode yfan | + node1 = yfan and + node2.asExpr() = yfan.asDefinition().(Assignment).getRValue() + ) + } + + /** + * Enforcing the check must occur in the same call context as the source, + * i.e., do not return from the source function and check in a caller. + */ + DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext } +} + +module YearAssignmentToLeapYearCheckFlow = + TaintTracking::Global; + +/** Does there exist a flow from the given YearFieldAccess to a Leap Year check or guard? */ +predicate isYearModifiedWithCheck(YearFieldAssignmentNode n) { + exists(YearAssignmentToLeapYearCheckFlow::PathNode src | + src.isSource() and + src.getNode() = n + ) + or + // If the time flows to a time conversion whose value/result is checked, + // assume the leap year is being handled. + exists(YearAssignmentToCheckedTimeConversionFlow::PathNode timeQualSrc | + timeQualSrc.isSource() and + timeQualSrc.getNode() = n + ) +} + +/** + * An expression which checks the value of a Month field `a->month == 1`. + */ +class MonthEqualityCheck extends EqualityOperation { + MonthEqualityCheck() { this.getAnOperand() instanceof MonthFieldAccess } + + Expr getExprCompared() { + exists(Expr e | + e = this.getAnOperand() and + not e instanceof MonthFieldAccess and + result = e + ) + } +} + +final class FinalMonthEqualityCheck = MonthEqualityCheck; + +class MonthEqualityCheckGuard extends GuardCondition, FinalMonthEqualityCheck { } + +/** + * Verifies if the expression is guarded by a check on the Month property of a date struct, that is NOT February. + */ +bindingset[e] +pragma[inline_late] +predicate isControlledByMonthEqualityCheckNonFebruary(Expr e) { + exists(MonthEqualityCheckGuard monthGuard | + monthGuard.controls(e.getBasicBlock(), true) and + not monthGuard.getExprCompared().getValueText() = "2" + ) +} + +/** + * Flow 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; + + predicate isSource(DataFlow::Node source, FlowState state) { + source instanceof YearFieldAssignmentNode and + state = false + } + + predicate isSink(DataFlow::Node sink, FlowState state) { + state = true and + ( + exists(IfStmt ifs | ifs.getCondition().getAChild*() = [sink.asExpr(), sink.asIndirectExpr()]) or - // If there is a data flow from the field that was modified to a function that seems to check for leap year - exists(VariableAccess vacheck, YearFieldAccess yfacheck, ChecksForLeapYearFunctionCall fc | - vacheck = var.getAnAccess() and - yfacheck.getQualifier() = vacheck and - LeapYearCheckFlow::flow(DataFlow::exprNode(yfacheck), DataFlow::exprNode(fc.getAnArgument())) + exists(ConditionalExpr ce | + ce.getCondition().getAChild*() = [sink.asExpr(), sink.asIndirectExpr()] + ) + or + exists(Loop l | l.getCondition().getAChild*() = [sink.asExpr(), sink.asIndirectExpr()]) + ) + or + state in [true, false] and + exists(Call c, TimeConversionFunction f | + f.isAutoLeapYearCorrecting() and + c.getTarget() = f and + c.getAnArgument().getAChild*() = [sink.asExpr(), sink.asIndirectExpr()] + ) + } + + predicate isAdditionalFlowStep( + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 + ) { + state1 in [true, false] and + state2 = true and + exists(Call c | + c.getTarget() instanceof TimeConversionFunction and + c.getAnArgument().getAChild*() = [node1.asExpr(), node1.asIndirectExpr()] and + node2.asExpr() = c + ) + } + + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + // flow from a YearFieldAccess to the qualifier + node2.asExpr() = node1.asExpr().(YearFieldAccess).getQualifier*() + or + node1.(YearFieldAssignmentNode).getYearFieldAccess().getQualifier() = node2.asExpr() + or + // Pass through any intermediate struct + exists(Assignment a, DataFlow::PostUpdateNode pun | + a.getLValue().(YearFieldAccess).getQualifier*() = pun.getPreUpdateNode().asExpr() and + a.getRValue() = node1.asExpr() and + node2.asExpr() = a.getLValue().(YearFieldAccess).getQualifier*() + ) + or + // flow from a year access qualifier to a year field + exists(YearFieldAccess yfa | node2.asExpr() = yfa and node1.asExpr() = yfa.getQualifier()) + } + + DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext } +} + +module YearAssignmentToCheckedTimeConversionFlow = + DataFlow::GlobalWithState; + +/** + * Finds flow from a parameter of a function to a leap year check. + * This is necessary to handle for scenarios like this: + * + * year = DANGEROUS_OP // source + * isLeap = isLeapYear(year); + * // logic based on isLeap + * struct.year = year; // sink + * + * In this case, we may flow a dangerous op to a year assignment, failing + * to barrier the flow through a leap year check, as the leap year check + * is nested, and dataflow does not progress down into the check and out. + * Instead, the point of this flow is to detect isLeapYear's argument + * is checked for leap year, making the isLeapYear call a barrier for + * the dangerous flow if we flow through the parameter identified to + * be checked. + */ +module ParameterToLeapYearCheckConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { exists(source.asParameter()) } + + predicate isSink(DataFlow::Node sink) { + exists(LeapYearGuardCondition lgc | + lgc.checkedYearAccess() = [sink.asExpr(), sink.asIndirectExpr()] + ) + } + + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + // flow from a YearFieldAccess to the qualifier + node2.asExpr() = node1.asExpr().(YearFieldAccess).getQualifier*() + or + // flow from a year access qualifier to a year field + exists(YearFieldAccess yfa | node2.asExpr() = yfa and node1.asExpr() = yfa.getQualifier()) + } + + /** + * Enforcing the check must occur in the same call context as the source, + * i.e., do not return from the source function and check in a caller. + */ + DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext } +} + +// NOTE: I do not believe taint flow is necessary here as we should +// be flowing directyly from some parameter to a leap year check. +module ParameterToLeapYearCheckFlow = DataFlow::Global; + +predicate isLeapYearCheckCall(Call c, Expr arg) { + exists(ParameterToLeapYearCheckFlow::PathNode src, Function f, int i | + src.isSource() and + f.getParameter(i) = src.getNode().asParameter() and + c.getTarget() = f and + c.getArgument(i) = arg + ) +} + +class LeapYearGuardCondition extends GuardCondition { + Expr yearSinkDiv4; + Expr yearSinkDiv100; + Expr yearSinkDiv400; + + LeapYearGuardCondition() { + exists( + LogicalAndExpr andExpr, LogicalOrExpr orExpr, GuardCondition div4Check, + GuardCondition div100Check, GuardCondition div400Check, GuardValue gv + | + // Cannonical case: + // form: `(year % 4 == 0) && (year % 100 != 0 || year % 400 == 0)` + // `!((year % 4 == 0) && (year % 100 != 0 || year % 400 == 0))` + // `!(year % 4) && (year % 100 || !(year % 400))` + // Also accepting `((year & 3) == 0) && (year % 100 != 0 || year % 400 == 0)` + // and `(year % 4 == 0) && (year % 100 > 0 || year % 400 == 0)` + this = andExpr and + andExpr.hasOperands(div4Check, orExpr) and + orExpr.hasOperands(div100Check, div400Check) and + ( + // year % 4 == 0 + exists(RemExpr e | + div4Check.comparesEq(e, 0, true, gv) and + e.getRightOperand().getValue().toInt() = 4 and + yearSinkDiv4 = e.getLeftOperand() + ) + or + // year & 3 == 0 + exists(BitwiseAndExpr e | + div4Check.comparesEq(e, 0, true, gv) and + e.getRightOperand().getValue().toInt() = 3 and + yearSinkDiv4 = e.getLeftOperand() + ) + ) and + exists(RemExpr e | + // year % 100 != 0 or year % 100 > 0 + ( + div100Check.comparesEq(e, 0, false, gv) or + div100Check.comparesLt(e, 1, false, gv) + ) and + e.getRightOperand().getValue().toInt() = 100 and + yearSinkDiv100 = e.getLeftOperand() + ) and + // year % 400 == 0 + exists(RemExpr e | + div400Check.comparesEq(e, 0, true, gv) and + e.getRightOperand().getValue().toInt() = 400 and + yearSinkDiv400 = e.getLeftOperand() ) or - // If there is a successor or predecessor that sets the month = 1 - exists(MonthFieldAccess mfa, AssignExpr ae | - mfa.getQualifier() = var.getAnAccess() and - mfa.isModified() and + // Inverted logic case: + // `year % 4 != 0 || (year % 100 == 0 && year % 400 != 0)` + // or `year & 3 != 0 || (year % 100 == 0 && year % 400 != 0)` + // also accepting `year % 4 > 0 || (year % 100 == 0 && year % 400 > 0)` + this = orExpr and + orExpr.hasOperands(div4Check, andExpr) and + andExpr.hasOperands(div100Check, div400Check) and + ( + // year % 4 != 0 or year % 4 > 0 + exists(RemExpr e | + ( + div4Check.comparesEq(e, 0, false, gv) + or + div4Check.comparesLt(e, 1, false, gv) + ) and + e.getRightOperand().getValue().toInt() = 4 and + yearSinkDiv4 = e.getLeftOperand() + ) + or + // year & 3 != 0 + exists(BitwiseAndExpr e | + div4Check.comparesEq(e, 0, false, gv) and + e.getRightOperand().getValue().toInt() = 3 and + yearSinkDiv4 = e.getLeftOperand() + ) + ) and + // year % 100 == 0 + exists(RemExpr e | + div100Check.comparesEq(e, 0, true, gv) and + e.getRightOperand().getValue().toInt() = 100 and + yearSinkDiv100 = e.getLeftOperand() + ) and + // year % 400 != 0 or year % 400 > 0 + exists(RemExpr e | ( - mfa.getBasicBlock() = yfa.getBasicBlock().getASuccessor*() or - yfa.getBasicBlock() = mfa.getBasicBlock().getASuccessor+() + div400Check.comparesEq(e, 0, false, gv) + or + div400Check.comparesLt(e, 1, false, gv) ) and - ae = mfa.getEnclosingElement() and - ae.getAnOperand().getValue().toInt() = 1 + e.getRightOperand().getValue().toInt() = 400 and + yearSinkDiv400 = e.getLeftOperand() + ) + ) + } + + Expr getYearSinkDiv4() { result = yearSinkDiv4 } + + Expr getYearSinkDiv100() { result = yearSinkDiv100 } + + Expr getYearSinkDiv400() { result = yearSinkDiv400 } + + /** + * The variable access that is used in all 3 components of the leap year check + * e.g., see getYearSinkDiv4/100/400.. + * If a field access is used, the qualifier and the field access are both returned + * in checked condition. + * NOTE: if the year is not checked using the same access in all 3 components, no result is returned. + * The typical case observed is a consistent variable access is used. If not, this may indicate a bug. + * We could check more accurately with a dataflow analysis, but this is likely sufficient for now. + */ + VariableAccess checkedYearAccess() { + exists(Variable var | + ( + this.getYearSinkDiv4().getAChild*() = var.getAnAccess() and + this.getYearSinkDiv100().getAChild*() = var.getAnAccess() and + this.getYearSinkDiv400().getAChild*() = var.getAnAccess() and + result = var.getAnAccess() and + ( + result = this.getYearSinkDiv4().getAChild*() or + result = this.getYearSinkDiv100().getAChild*() or + result = this.getYearSinkDiv400().getAChild*() + ) ) ) + } +} + +/** + * A difficult case to detect is if a year modification is tied to a month or day modification + * and the month or day is safe for leap year. + * e.g., + * year++; + * month = 1; + * // alternative: day = 15; + * ... values eventually used in the same time struct + * If this is even more challenging if the struct the values end up in are not + * local (set inter-procedurally). + * This flow flows constants 1-31 to a month or day assignment. + * It is assumed a user of this flow will check if the month/day source and month/day sink + * are in the same basic blocks as a year modification source and a year modification sink. + * It is also assumed a user will check if the constant source is a value that is ignorable + * e.g., if it is 2 and the sink is a month assignment, then it isn't ignorable or + * if the value is < 27 and is a day assignment, it is likely ignorable + * + * Obviously this does not handle all conditions (e.g., the month set in another block). + * It is meant to capture the most common cases of false positives. + */ +module CandidateConstantToDayOrMonthAssignmentConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { + source.asExpr().getValue().toInt() in [1 .. 31] and + ( + exists(Assignment a | a.getRValue() = source.asExpr()) + or + exists(Call c | c.getAnArgument() = source.asExpr()) + ) + } + + predicate isSink(DataFlow::Node sink) { + exists(Assignment a | + (a.getLValue() instanceof MonthFieldAccess or a.getLValue() instanceof DayFieldAccess) and + a.getRValue() = sink.asExpr() + ) + } +} + +// NOTE: only data flow here (no taint tracking) as we want the exact +// constant flowing to the month assignment +module CandidateConstantToDayOrMonthAssignmentFlow = + DataFlow::Global; + +/** + * The value that the assignment resolves to doesn't represent February, + * and/or if it represents a day, is a 'safe' day (meaning the 27th or prior). + */ +bindingset[dayOrMonthValSrcExpr] +predicate isSafeValueForAssignmentOfMonthOrDayValue(Assignment a, Expr dayOrMonthValSrcExpr) { + a.getLValue() instanceof MonthFieldAccess and + dayOrMonthValSrcExpr.getValue().toInt() != 2 + or + a.getLValue() instanceof DayFieldAccess and + dayOrMonthValSrcExpr.getValue().toInt() <= 27 +} + +import OperationToYearAssignmentFlow::PathGraph + +from OperationToYearAssignmentFlow::PathNode src, OperationToYearAssignmentFlow::PathNode sink +where + OperationToYearAssignmentFlow::flowPath(src, sink) and + // Check if a month is set in the same block as the year operation source + // and the month value would indicate its set to any other month than february. + // Finds if the source year node is in the same block as a source month block + // and if the same for the sinks. + not exists(DataFlow::Node dayOrMonthValSrc, DataFlow::Node dayOrMonthValSink, Assignment a | + CandidateConstantToDayOrMonthAssignmentFlow::flow(dayOrMonthValSrc, dayOrMonthValSink) and + a.getRValue() = dayOrMonthValSink.asExpr() and + dayOrMonthValSink.getBasicBlock() = sink.getNode().getBasicBlock() and + exists(IRBlock dayOrMonthValBB | + dayOrMonthValBB = dayOrMonthValSrc.getBasicBlock() and + // The source of the day is set in the same block as the source for the year + // or the source for the day is set in the same block as the sink for the year + dayOrMonthValBB in [ + src.getNode().getBasicBlock(), + sink.getNode().getBasicBlock() + ] + ) and + isSafeValueForAssignmentOfMonthOrDayValue(a, dayOrMonthValSrc.asExpr()) ) -select yfa, - "Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found.", - yfa.getTarget(), yfa.getTarget().toString(), var, var.toString() +select sink, src, sink, + "Year field has been modified, but no appropriate check for LeapYear was found." diff --git a/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql b/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql index af02a2814a20..639eaf54e845 100644 --- a/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql +++ b/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql @@ -44,21 +44,6 @@ class SafeTimeGatheringFunction extends Function { } } -/** - * This list of APIs should check for the return value to detect problems during the conversion. - */ -class TimeConversionFunction extends Function { - TimeConversionFunction() { - this.getQualifiedName() = - [ - "FileTimeToSystemTime", "SystemTimeToFileTime", "SystemTimeToTzSpecificLocalTime", - "SystemTimeToTzSpecificLocalTimeEx", "TzSpecificLocalTimeToSystemTime", - "TzSpecificLocalTimeToSystemTimeEx", "RtlLocalTimeToSystemTime", - "RtlTimeToSecondsSince1970", "_mkgmtime" - ] - } -} - from FunctionCall fcall, TimeConversionFunction trf, Variable var where fcall = trf.getACallToThisFunction() and From d9feadcfec50bfceee158734df79da6d408f66eb Mon Sep 17 00:00:00 2001 From: "REDMOND\\brodes" Date: Fri, 6 Feb 2026 15:45:07 -0500 Subject: [PATCH 3/8] C++. Accept test changes. One false positive introduced, and one false negative remains. --- ...ckedLeapYearAfterYearModification.expected | 199 +++++++++++++----- ...heckedReturnValueForTimeFunctions.expected | 1 + 2 files changed, 142 insertions(+), 58 deletions(-) diff --git a/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedLeapYearAfterYearModification.expected b/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedLeapYearAfterYearModification.expected index 8fdc1339aa1e..64b21f1f9e42 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedLeapYearAfterYearModification.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedLeapYearAfterYearModification.expected @@ -1,60 +1,143 @@ #select -| test.cpp:422:5:422:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:417:13:417:14 | st | st | -| test.cpp:440:5:440:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:435:13:435:14 | st | st | -| test.cpp:456:6:456:10 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:451:62:451:63 | st | st | -| test.cpp:647:5:647:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:643:13:643:14 | st | st | -| test.cpp:665:5:665:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:661:13:661:14 | st | st | -| test.cpp:681:5:681:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:677:13:677:14 | st | st | -| test.cpp:792:11:792:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:789:12:789:19 | timeinfo | timeinfo | -| test.cpp:813:11:813:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:806:12:806:19 | timeinfo | timeinfo | -| test.cpp:818:5:818:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:807:13:807:14 | st | st | -| test.cpp:954:6:954:10 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:947:14:947:15 | st | st | -| test.cpp:972:6:972:10 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:965:14:965:15 | st | st | -| test.cpp:990:6:990:10 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:984:14:984:15 | st | st | -| test.cpp:1077:5:1077:9 | wYear | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:12:7:12:11 | wYear | wYear | test.cpp:1071:13:1071:14 | st | st | -| test.cpp:1135:9:1135:15 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:92:17:92:23 | tm_year | tm_year | test.cpp:1126:35:1126:36 | tm | tm | -| test.cpp:1137:9:1137:15 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:92:17:92:23 | tm_year | tm_year | test.cpp:1126:35:1126:36 | tm | tm | -| test.cpp:1591:11:1591:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1590:41:1590:48 | timeinfo | timeinfo | -| test.cpp:1599:11:1599:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1598:41:1598:48 | timeinfo | timeinfo | -| test.cpp:1645:11:1645:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1644:57:1644:64 | timeinfo | timeinfo | -| test.cpp:1656:11:1656:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1655:57:1655:64 | timeinfo | timeinfo | -| test.cpp:1667:11:1667:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1666:30:1666:37 | timeinfo | timeinfo | -| test.cpp:1678:11:1678:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1677:30:1677:37 | timeinfo | timeinfo | -| test.cpp:1690:11:1690:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1689:30:1689:37 | timeinfo | timeinfo | -| test.cpp:1702:11:1702:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1701:30:1701:37 | timeinfo | timeinfo | -| test.cpp:1712:11:1712:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1711:30:1711:37 | timeinfo | timeinfo | -| test.cpp:1723:11:1723:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1722:40:1722:47 | timeinfo | timeinfo | -| test.cpp:1758:11:1758:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1757:45:1757:52 | timeinfo | timeinfo | -| test.cpp:1763:11:1763:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1762:45:1762:52 | timeinfo | timeinfo | -| test.cpp:1792:11:1792:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1788:38:1788:45 | timeinfo | timeinfo | -| test.cpp:1799:11:1799:17 | tm_year | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | test.cpp:58:6:58:12 | tm_year | tm_year | test.cpp:1798:36:1798:43 | timeinfo | timeinfo | +| test.cpp:422:2:422:14 | ... += ... | test.cpp:422:2:422:14 | ... += ... | test.cpp:422:2:422:14 | ... += ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:440:2:440:11 | ... ++ | test.cpp:440:2:440:11 | ... ++ | test.cpp:440:2:440:11 | ... ++ | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:456:2:456:12 | ... ++ | test.cpp:456:2:456:12 | ... ++ | test.cpp:456:2:456:12 | ... ++ | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:681:2:681:23 | ... += ... | test.cpp:681:2:681:23 | ... += ... | test.cpp:681:2:681:23 | ... += ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:769:2:769:23 | ... -= ... | test.cpp:769:2:769:23 | ... -= ... | test.cpp:769:2:769:23 | ... -= ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:813:2:813:40 | ... = ... | test.cpp:813:21:813:40 | ... + ... | test.cpp:813:2:813:40 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:818:2:818:24 | ... = ... | test.cpp:818:13:818:24 | ... + ... | test.cpp:818:2:818:24 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:954:3:954:25 | ... = ... | test.cpp:954:14:954:25 | ... + ... | test.cpp:954:3:954:25 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:972:3:972:12 | ... ++ | test.cpp:972:3:972:12 | ... ++ | test.cpp:972:3:972:12 | ... ++ | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1077:2:1077:11 | ... ++ | test.cpp:1077:2:1077:11 | ... ++ | test.cpp:1077:2:1077:11 | ... ++ | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1097:16:1097:23 | increment_arg output argument | test.cpp:1085:2:1085:4 | ... ++ | test.cpp:1097:16:1097:23 | increment_arg output argument | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1101:27:1101:35 | increment_arg_by_pointer output argument | test.cpp:1089:2:1089:7 | ... ++ | test.cpp:1101:27:1101:35 | increment_arg_by_pointer output argument | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1206:2:1206:19 | ... = ... | test.cpp:1204:2:1204:15 | ... += ... | test.cpp:1206:2:1206:19 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1245:2:1245:28 | ... = ... | test.cpp:1245:16:1245:28 | ... + ... | test.cpp:1245:2:1245:28 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1260:2:1260:28 | ... = ... | test.cpp:1260:16:1260:28 | ... + ... | test.cpp:1260:2:1260:28 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1274:2:1274:28 | ... = ... | test.cpp:1274:16:1274:28 | ... + ... | test.cpp:1274:2:1274:28 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1288:2:1288:26 | ... = ... | test.cpp:1288:14:1288:26 | ... + ... | test.cpp:1288:2:1288:26 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1302:2:1302:26 | ... = ... | test.cpp:1302:14:1302:26 | ... + ... | test.cpp:1302:2:1302:26 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1308:2:1308:28 | ... = ... | test.cpp:1308:16:1308:28 | ... + ... | test.cpp:1308:2:1308:28 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1320:2:1320:28 | ... = ... | test.cpp:1320:16:1320:28 | ... + ... | test.cpp:1320:2:1320:28 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1333:2:1333:26 | ... = ... | test.cpp:1333:14:1333:26 | ... + ... | test.cpp:1333:2:1333:26 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1345:2:1345:26 | ... = ... | test.cpp:1345:14:1345:26 | ... + ... | test.cpp:1345:2:1345:26 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1387:2:1387:17 | ... = ... | test.cpp:1478:12:1478:17 | ... + ... | test.cpp:1387:2:1387:17 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1387:2:1387:17 | ... = ... | test.cpp:1492:9:1492:16 | ... + ... | test.cpp:1387:2:1387:17 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1387:2:1387:17 | ... = ... | test.cpp:1504:9:1504:16 | ... + ... | test.cpp:1387:2:1387:17 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1561:2:1561:15 | ... = ... | test.cpp:1558:2:1558:10 | ... += ... | test.cpp:1561:2:1561:15 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1591:2:1591:22 | ... += ... | test.cpp:1591:2:1591:22 | ... += ... | test.cpp:1591:2:1591:22 | ... += ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1599:2:1599:22 | ... += ... | test.cpp:1599:2:1599:22 | ... += ... | test.cpp:1599:2:1599:22 | ... += ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1678:2:1678:22 | ... += ... | test.cpp:1678:2:1678:22 | ... += ... | test.cpp:1678:2:1678:22 | ... += ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1690:2:1690:22 | ... += ... | test.cpp:1690:2:1690:22 | ... += ... | test.cpp:1690:2:1690:22 | ... += ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1723:2:1723:22 | ... += ... | test.cpp:1723:2:1723:22 | ... += ... | test.cpp:1723:2:1723:22 | ... += ... | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1799:2:1799:22 | ... += ... | test.cpp:1799:2:1799:22 | ... += ... | test.cpp:1799:2:1799:22 | ... += ... | Year field has been modified, but no appropriate check for LeapYear was found. | +edges +| test.cpp:813:21:813:40 | ... + ... | test.cpp:813:2:813:40 | ... = ... | provenance | | +| test.cpp:818:13:818:24 | ... + ... | test.cpp:818:2:818:24 | ... = ... | provenance | | +| test.cpp:954:14:954:25 | ... + ... | test.cpp:954:3:954:25 | ... = ... | provenance | | +| test.cpp:1084:26:1084:26 | *x | test.cpp:1097:16:1097:23 | increment_arg output argument | provenance | | +| test.cpp:1085:2:1085:4 | ... ++ | test.cpp:1084:26:1084:26 | *x | provenance | | +| test.cpp:1088:37:1088:37 | *x | test.cpp:1101:27:1101:35 | increment_arg_by_pointer output argument | provenance | | +| test.cpp:1089:2:1089:7 | ... ++ | test.cpp:1088:37:1088:37 | *x | provenance | | +| test.cpp:1204:2:1204:15 | ... += ... | test.cpp:1206:2:1206:19 | ... = ... | provenance | | +| test.cpp:1245:16:1245:28 | ... + ... | test.cpp:1245:2:1245:28 | ... = ... | provenance | | +| test.cpp:1260:16:1260:28 | ... + ... | test.cpp:1260:2:1260:28 | ... = ... | provenance | | +| test.cpp:1274:16:1274:28 | ... + ... | test.cpp:1274:2:1274:28 | ... = ... | provenance | | +| test.cpp:1288:14:1288:26 | ... + ... | test.cpp:1288:2:1288:26 | ... = ... | provenance | | +| test.cpp:1302:14:1302:26 | ... + ... | test.cpp:1302:2:1302:26 | ... = ... | provenance | | +| test.cpp:1308:16:1308:28 | ... + ... | test.cpp:1308:2:1308:28 | ... = ... | provenance | | +| test.cpp:1320:16:1320:28 | ... + ... | test.cpp:1320:2:1320:28 | ... = ... | provenance | | +| test.cpp:1333:14:1333:26 | ... + ... | test.cpp:1333:2:1333:26 | ... = ... | provenance | | +| test.cpp:1345:14:1345:26 | ... + ... | test.cpp:1345:2:1345:26 | ... = ... | provenance | | +| test.cpp:1384:20:1384:23 | year | test.cpp:1387:2:1387:17 | ... = ... | provenance | | +| test.cpp:1397:15:1397:22 | ... + ... | test.cpp:1397:3:1397:22 | ... = ... | provenance | | +| test.cpp:1402:12:1402:17 | ... + ... | test.cpp:1384:20:1384:23 | year | provenance | | +| test.cpp:1411:15:1411:22 | ... + ... | test.cpp:1411:3:1411:22 | ... = ... | provenance | | +| test.cpp:1421:3:1421:20 | ... = ... | test.cpp:1423:12:1423:18 | yeartmp | provenance | | +| test.cpp:1421:13:1421:20 | ... + ... | test.cpp:1421:3:1421:20 | ... = ... | provenance | | +| test.cpp:1423:12:1423:18 | yeartmp | test.cpp:1384:20:1384:23 | year | provenance | | +| test.cpp:1466:15:1466:22 | ... + ... | test.cpp:1466:3:1466:22 | ... = ... | provenance | | +| test.cpp:1471:12:1471:17 | ... + ... | test.cpp:1384:20:1384:23 | year | provenance | | +| test.cpp:1478:12:1478:17 | ... + ... | test.cpp:1384:20:1384:23 | year | provenance | | +| test.cpp:1492:2:1492:16 | ... = ... | test.cpp:1496:3:1496:18 | ... = ... | provenance | | +| test.cpp:1492:2:1492:16 | ... = ... | test.cpp:1501:12:1501:15 | year | provenance | | +| test.cpp:1492:9:1492:16 | ... + ... | test.cpp:1492:2:1492:16 | ... = ... | provenance | | +| test.cpp:1501:12:1501:15 | year | test.cpp:1384:20:1384:23 | year | provenance | | +| test.cpp:1504:2:1504:16 | ... = ... | test.cpp:1510:12:1510:15 | year | provenance | | +| test.cpp:1504:9:1504:16 | ... + ... | test.cpp:1504:2:1504:16 | ... = ... | provenance | | +| test.cpp:1510:12:1510:15 | year | test.cpp:1384:20:1384:23 | year | provenance | | +| test.cpp:1558:2:1558:10 | ... += ... | test.cpp:1561:2:1561:15 | ... = ... | provenance | | +nodes +| test.cpp:422:2:422:14 | ... += ... | semmle.label | ... += ... | +| test.cpp:440:2:440:11 | ... ++ | semmle.label | ... ++ | +| test.cpp:456:2:456:12 | ... ++ | semmle.label | ... ++ | +| test.cpp:482:3:482:12 | ... ++ | semmle.label | ... ++ | +| test.cpp:681:2:681:23 | ... += ... | semmle.label | ... += ... | +| test.cpp:769:2:769:23 | ... -= ... | semmle.label | ... -= ... | +| test.cpp:813:2:813:40 | ... = ... | semmle.label | ... = ... | +| test.cpp:813:21:813:40 | ... + ... | semmle.label | ... + ... | +| test.cpp:818:2:818:24 | ... = ... | semmle.label | ... = ... | +| test.cpp:818:13:818:24 | ... + ... | semmle.label | ... + ... | +| test.cpp:875:4:875:15 | ... ++ | semmle.label | ... ++ | +| test.cpp:954:3:954:25 | ... = ... | semmle.label | ... = ... | +| test.cpp:954:14:954:25 | ... + ... | semmle.label | ... + ... | +| test.cpp:972:3:972:12 | ... ++ | semmle.label | ... ++ | +| test.cpp:1077:2:1077:11 | ... ++ | semmle.label | ... ++ | +| test.cpp:1084:26:1084:26 | *x | semmle.label | *x | +| test.cpp:1085:2:1085:4 | ... ++ | semmle.label | ... ++ | +| test.cpp:1088:37:1088:37 | *x | semmle.label | *x | +| test.cpp:1089:2:1089:7 | ... ++ | semmle.label | ... ++ | +| test.cpp:1097:16:1097:23 | increment_arg output argument | semmle.label | increment_arg output argument | +| test.cpp:1101:27:1101:35 | increment_arg_by_pointer output argument | semmle.label | increment_arg_by_pointer output argument | +| test.cpp:1204:2:1204:15 | ... += ... | semmle.label | ... += ... | +| test.cpp:1206:2:1206:19 | ... = ... | semmle.label | ... = ... | +| test.cpp:1245:2:1245:28 | ... = ... | semmle.label | ... = ... | +| test.cpp:1245:16:1245:28 | ... + ... | semmle.label | ... + ... | +| test.cpp:1260:2:1260:28 | ... = ... | semmle.label | ... = ... | +| test.cpp:1260:16:1260:28 | ... + ... | semmle.label | ... + ... | +| test.cpp:1274:2:1274:28 | ... = ... | semmle.label | ... = ... | +| test.cpp:1274:16:1274:28 | ... + ... | semmle.label | ... + ... | +| test.cpp:1288:2:1288:26 | ... = ... | semmle.label | ... = ... | +| test.cpp:1288:14:1288:26 | ... + ... | semmle.label | ... + ... | +| test.cpp:1302:2:1302:26 | ... = ... | semmle.label | ... = ... | +| test.cpp:1302:14:1302:26 | ... + ... | semmle.label | ... + ... | +| test.cpp:1308:2:1308:28 | ... = ... | semmle.label | ... = ... | +| test.cpp:1308:16:1308:28 | ... + ... | semmle.label | ... + ... | +| test.cpp:1320:2:1320:28 | ... = ... | semmle.label | ... = ... | +| test.cpp:1320:16:1320:28 | ... + ... | semmle.label | ... + ... | +| test.cpp:1333:2:1333:26 | ... = ... | semmle.label | ... = ... | +| test.cpp:1333:14:1333:26 | ... + ... | semmle.label | ... + ... | +| test.cpp:1345:2:1345:26 | ... = ... | semmle.label | ... = ... | +| test.cpp:1345:14:1345:26 | ... + ... | semmle.label | ... + ... | +| test.cpp:1384:20:1384:23 | year | semmle.label | year | +| test.cpp:1387:2:1387:17 | ... = ... | semmle.label | ... = ... | +| test.cpp:1397:3:1397:22 | ... = ... | semmle.label | ... = ... | +| test.cpp:1397:15:1397:22 | ... + ... | semmle.label | ... + ... | +| test.cpp:1402:12:1402:17 | ... + ... | semmle.label | ... + ... | +| test.cpp:1411:3:1411:22 | ... = ... | semmle.label | ... = ... | +| test.cpp:1411:15:1411:22 | ... + ... | semmle.label | ... + ... | +| test.cpp:1421:3:1421:20 | ... = ... | semmle.label | ... = ... | +| test.cpp:1421:13:1421:20 | ... + ... | semmle.label | ... + ... | +| test.cpp:1423:12:1423:18 | yeartmp | semmle.label | yeartmp | +| test.cpp:1466:3:1466:22 | ... = ... | semmle.label | ... = ... | +| test.cpp:1466:15:1466:22 | ... + ... | semmle.label | ... + ... | +| test.cpp:1471:12:1471:17 | ... + ... | semmle.label | ... + ... | +| test.cpp:1478:12:1478:17 | ... + ... | semmle.label | ... + ... | +| test.cpp:1492:2:1492:16 | ... = ... | semmle.label | ... = ... | +| test.cpp:1492:9:1492:16 | ... + ... | semmle.label | ... + ... | +| test.cpp:1496:3:1496:18 | ... = ... | semmle.label | ... = ... | +| test.cpp:1501:12:1501:15 | year | semmle.label | year | +| test.cpp:1504:2:1504:16 | ... = ... | semmle.label | ... = ... | +| test.cpp:1504:9:1504:16 | ... + ... | semmle.label | ... + ... | +| test.cpp:1510:12:1510:15 | year | semmle.label | year | +| test.cpp:1558:2:1558:10 | ... += ... | semmle.label | ... += ... | +| test.cpp:1561:2:1561:15 | ... = ... | semmle.label | ... = ... | +| test.cpp:1591:2:1591:22 | ... += ... | semmle.label | ... += ... | +| test.cpp:1599:2:1599:22 | ... += ... | semmle.label | ... += ... | +| test.cpp:1678:2:1678:22 | ... += ... | semmle.label | ... += ... | +| test.cpp:1690:2:1690:22 | ... += ... | semmle.label | ... += ... | +| test.cpp:1723:2:1723:22 | ... += ... | semmle.label | ... += ... | +| test.cpp:1799:2:1799:22 | ... += ... | semmle.label | ... += ... | +subpaths testFailures -| test.cpp:422:5:422:9 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:440:5:440:9 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:456:6:456:10 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:647:5:647:9 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:665:5:665:9 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:681:5:681:9 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:792:11:792:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:813:11:813:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:818:5:818:9 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:954:6:954:10 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:972:6:972:10 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:990:6:990:10 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:1077:5:1077:9 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:1135:9:1135:15 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:1137:9:1137:15 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:1591:11:1591:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:1599:11:1599:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:1645:11:1645:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:1656:11:1656:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:1667:11:1667:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:1678:11:1678:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:1690:11:1690:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:1702:11:1702:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:1712:11:1712:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:1723:11:1723:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:1758:11:1758:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:1763:11:1763:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:1792:11:1792:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | -| test.cpp:1799:11:1799:17 | Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found. | Unexpected result: Alert | +| test.cpp:1155:29:1155:98 | // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] | Missing result: Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedReturnValueForTimeFunctions.expected b/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedReturnValueForTimeFunctions.expected index e893ae1fff0e..f572f386be2b 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedReturnValueForTimeFunctions.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedReturnValueForTimeFunctions.expected @@ -4,3 +4,4 @@ | test.cpp:956:3:956:22 | call to SystemTimeToFileTime | Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe. | test.cpp:101:1:101:20 | SystemTimeToFileTime | SystemTimeToFileTime | test.cpp:947:14:947:15 | st | st | | test.cpp:974:3:974:22 | call to SystemTimeToFileTime | Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe. | test.cpp:101:1:101:20 | SystemTimeToFileTime | SystemTimeToFileTime | test.cpp:965:14:965:15 | st | st | | test.cpp:1081:2:1081:21 | call to SystemTimeToFileTime | Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe. | test.cpp:101:1:101:20 | SystemTimeToFileTime | SystemTimeToFileTime | test.cpp:1071:13:1071:14 | st | st | +| test.cpp:1794:2:1794:7 | call to mktime | Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe. | test.cpp:166:12:166:17 | mktime | mktime | test.cpp:1788:38:1788:45 | timeinfo | timeinfo | From ca18179bd2d36f9cf3ad28b70a845bfada96dbed Mon Sep 17 00:00:00 2001 From: "REDMOND\\brodes" Date: Fri, 6 Feb 2026 16:07:07 -0500 Subject: [PATCH 4/8] C++: Correct false positive. Only TimeConversionFunction that do not auto correct for leap year should be considered. --- .../Leap Year/UncheckedReturnValueForTimeFunctions.ql | 1 + 1 file changed, 1 insertion(+) diff --git a/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql b/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql index 639eaf54e845..8e2d6e9d10fe 100644 --- a/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql +++ b/cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql @@ -46,6 +46,7 @@ class SafeTimeGatheringFunction extends Function { from FunctionCall fcall, TimeConversionFunction trf, Variable var where + not trf.isAutoLeapYearCorrecting() and fcall = trf.getACallToThisFunction() and fcall instanceof ExprInVoidContext and var.getUnderlyingType() instanceof UnpackedTimeType and From a534d26449e28c11ed671abb3cb3a2b1c3de4742 Mon Sep 17 00:00:00 2001 From: "REDMOND\\brodes" Date: Fri, 6 Feb 2026 16:07:44 -0500 Subject: [PATCH 5/8] C++: Accept test changes. --- .../UncheckedReturnValueForTimeFunctions.expected | 1 - 1 file changed, 1 deletion(-) diff --git a/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedReturnValueForTimeFunctions.expected b/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedReturnValueForTimeFunctions.expected index f572f386be2b..e893ae1fff0e 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedReturnValueForTimeFunctions.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedReturnValueForTimeFunctions.expected @@ -4,4 +4,3 @@ | test.cpp:956:3:956:22 | call to SystemTimeToFileTime | Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe. | test.cpp:101:1:101:20 | SystemTimeToFileTime | SystemTimeToFileTime | test.cpp:947:14:947:15 | st | st | | test.cpp:974:3:974:22 | call to SystemTimeToFileTime | Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe. | test.cpp:101:1:101:20 | SystemTimeToFileTime | SystemTimeToFileTime | test.cpp:965:14:965:15 | st | st | | test.cpp:1081:2:1081:21 | call to SystemTimeToFileTime | Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe. | test.cpp:101:1:101:20 | SystemTimeToFileTime | SystemTimeToFileTime | test.cpp:1071:13:1071:14 | st | st | -| test.cpp:1794:2:1794:7 | call to mktime | Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe. | test.cpp:166:12:166:17 | mktime | mktime | test.cpp:1788:38:1788:45 | timeinfo | timeinfo | From 2b806ad6fdebb124c9f653b958917503fdcfe094 Mon Sep 17 00:00:00 2001 From: "REDMOND\\brodes" Date: Fri, 6 Feb 2026 16:10:04 -0500 Subject: [PATCH 6/8] C++: Add missing DateTime models for PTIME_FIELDS and TIME_FIELDS --- .../lib/semmle/code/cpp/commons/DateTime.qll | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/cpp/ql/lib/semmle/code/cpp/commons/DateTime.qll b/cpp/ql/lib/semmle/code/cpp/commons/DateTime.qll index c67bf7cf96e3..a40221610763 100644 --- a/cpp/ql/lib/semmle/code/cpp/commons/DateTime.qll +++ b/cpp/ql/lib/semmle/code/cpp/commons/DateTime.qll @@ -14,7 +14,9 @@ class PackedTimeType extends Type { } } -private predicate timeType(string typeName) { typeName = ["_SYSTEMTIME", "SYSTEMTIME", "tm"] } +private predicate timeType(string typeName) { + typeName = ["_SYSTEMTIME", "SYSTEMTIME", "tm", "TIME_FIELDS", "_TIME_FIELDS", "PTIME_FIELDS"] +} /** * A type that is used to represent times and dates in an 'unpacked' form, that is, @@ -95,3 +97,24 @@ class StructTmMonthFieldAccess extends MonthFieldAccess { class StructTmYearFieldAccess extends YearFieldAccess { StructTmYearFieldAccess() { this.getTarget().getName() = "tm_year" } } + +/** + * A `DayFieldAccess` for the `TIME_FIELDS` struct. + */ +class TimeFieldsDayFieldAccess extends DayFieldAccess { + TimeFieldsDayFieldAccess() { this.getTarget().getName() = "Day" } +} + +/** + * A `MonthFieldAccess` for the `TIME_FIELDS` struct. + */ +class TimeFieldsMonthFieldAccess extends MonthFieldAccess { + TimeFieldsMonthFieldAccess() { this.getTarget().getName() = "Month" } +} + +/** + * A `YearFieldAccess` for the `TIME_FIELDS` struct. + */ +class TimeFieldsYearFieldAccess extends YearFieldAccess { + TimeFieldsYearFieldAccess() { this.getTarget().getName() = "Year" } +} From 36cc20989c22d646382d84926c7c0e65056243ef Mon Sep 17 00:00:00 2001 From: "REDMOND\\brodes" Date: Fri, 6 Feb 2026 16:11:51 -0500 Subject: [PATCH 7/8] C++: Accept test changes (removing false negative) --- .../UncheckedLeapYearAfterYearModification.expected | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedLeapYearAfterYearModification.expected b/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedLeapYearAfterYearModification.expected index 64b21f1f9e42..35a635ba903e 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedLeapYearAfterYearModification.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedLeapYearAfterYearModification.expected @@ -11,6 +11,7 @@ | test.cpp:1077:2:1077:11 | ... ++ | test.cpp:1077:2:1077:11 | ... ++ | test.cpp:1077:2:1077:11 | ... ++ | Year field has been modified, but no appropriate check for LeapYear was found. | | test.cpp:1097:16:1097:23 | increment_arg output argument | test.cpp:1085:2:1085:4 | ... ++ | test.cpp:1097:16:1097:23 | increment_arg output argument | Year field has been modified, but no appropriate check for LeapYear was found. | | test.cpp:1101:27:1101:35 | increment_arg_by_pointer output argument | test.cpp:1089:2:1089:7 | ... ++ | test.cpp:1101:27:1101:35 | increment_arg_by_pointer output argument | Year field has been modified, but no appropriate check for LeapYear was found. | +| test.cpp:1155:2:1155:26 | ... = ... | test.cpp:1155:14:1155:26 | ... - ... | test.cpp:1155:2:1155:26 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. | | test.cpp:1206:2:1206:19 | ... = ... | test.cpp:1204:2:1204:15 | ... += ... | test.cpp:1206:2:1206:19 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. | | test.cpp:1245:2:1245:28 | ... = ... | test.cpp:1245:16:1245:28 | ... + ... | test.cpp:1245:2:1245:28 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. | | test.cpp:1260:2:1260:28 | ... = ... | test.cpp:1260:16:1260:28 | ... + ... | test.cpp:1260:2:1260:28 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. | @@ -39,6 +40,7 @@ edges | test.cpp:1085:2:1085:4 | ... ++ | test.cpp:1084:26:1084:26 | *x | provenance | | | test.cpp:1088:37:1088:37 | *x | test.cpp:1101:27:1101:35 | increment_arg_by_pointer output argument | provenance | | | test.cpp:1089:2:1089:7 | ... ++ | test.cpp:1088:37:1088:37 | *x | provenance | | +| test.cpp:1155:14:1155:26 | ... - ... | test.cpp:1155:2:1155:26 | ... = ... | provenance | | | test.cpp:1204:2:1204:15 | ... += ... | test.cpp:1206:2:1206:19 | ... = ... | provenance | | | test.cpp:1245:16:1245:28 | ... + ... | test.cpp:1245:2:1245:28 | ... = ... | provenance | | | test.cpp:1260:16:1260:28 | ... + ... | test.cpp:1260:2:1260:28 | ... = ... | provenance | | @@ -89,6 +91,8 @@ nodes | test.cpp:1089:2:1089:7 | ... ++ | semmle.label | ... ++ | | test.cpp:1097:16:1097:23 | increment_arg output argument | semmle.label | increment_arg output argument | | test.cpp:1101:27:1101:35 | increment_arg_by_pointer output argument | semmle.label | increment_arg_by_pointer output argument | +| test.cpp:1155:2:1155:26 | ... = ... | semmle.label | ... = ... | +| test.cpp:1155:14:1155:26 | ... - ... | semmle.label | ... - ... | | test.cpp:1204:2:1204:15 | ... += ... | semmle.label | ... += ... | | test.cpp:1206:2:1206:19 | ... = ... | semmle.label | ... = ... | | test.cpp:1245:2:1245:28 | ... = ... | semmle.label | ... = ... | @@ -139,5 +143,3 @@ nodes | test.cpp:1723:2:1723:22 | ... += ... | semmle.label | ... += ... | | test.cpp:1799:2:1799:22 | ... += ... | semmle.label | ... += ... | subpaths -testFailures -| test.cpp:1155:29:1155:98 | // $ Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] | Missing result: Alert[cpp/leap-year/unchecked-after-arithmetic-year-modification] | From 1796bc0abbb43ca8f45e2da7109c4bccf6b48cfa Mon Sep 17 00:00:00 2001 From: "REDMOND\\brodes" Date: Fri, 6 Feb 2026 16:19:11 -0500 Subject: [PATCH 8/8] C++: Add change note. --- .../2026-02-06-UncheckedLeapYearAfterModification_Refactor | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 cpp/ql/lib/change-notes/2026-02-06-UncheckedLeapYearAfterModification_Refactor diff --git a/cpp/ql/lib/change-notes/2026-02-06-UncheckedLeapYearAfterModification_Refactor b/cpp/ql/lib/change-notes/2026-02-06-UncheckedLeapYearAfterModification_Refactor new file mode 100644 index 000000000000..3d0f71c5a65f --- /dev/null +++ b/cpp/ql/lib/change-notes/2026-02-06-UncheckedLeapYearAfterModification_Refactor @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Refactor of UncheckedLeapYearAfterYearModification.ql to address large numbers of false positives. Reduced alerts from 40k to 2k. \ No newline at end of file