-
Notifications
You must be signed in to change notification settings - Fork 1.9k
C++: Refactor of UncheckedLeapYearAfterModification #21292
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
C++: Refactor of UncheckedLeapYearAfterModification #21292
Conversation
…cks (UncheckedLeapYearAfterYearModification). Switch to using 'postprocess' for unit tests.
…ion. 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.
…e negative remains.
…auto correct for leap year should be considered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Refactors the UncheckedLeapYearAfterYearModification CodeQL query to significantly reduce false positives by introducing more precise flow/guard modeling and expanded heuristics around conversions and “ignorable” operations.
Changes:
- Reworked
UncheckedLeapYearAfterYearModification.qlinto a path-problem with new taint/dataflow-based modeling and multiple false-positive suppression heuristics. - Centralized/expanded time conversion API modeling in
LeapYear.qlland updated related query/tests/expected outputs accordingly. - Extended date/time type support (e.g.,
TIME_FIELDS) and added release change notes.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/test.cpp | Adds many new positive/negative/edge test scenarios for the refactored query. |
| cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedReturnValueForTimeFunctions.expected | Updates expected results to match new line numbers/coverage. |
| cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedLeapYearAfterYearModification.qlref | Switches to inline-expectations postprocessing. |
| cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/UncheckedLeapYearAfterYearModification.expected | Updates expected results to reflect path-problem output and new test suite. |
| cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql | Refines conversion-return checking to exclude auto-leap-year-correcting APIs. |
| cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql | Full refactor: new flow configurations, guard recognition, and suppression heuristics. |
| cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll | Moves/expands TimeConversionFunction modeling and adds auto-correcting classification. |
| cpp/ql/lib/semmle/code/cpp/commons/DateTime.qll | Expands recognized unpacked time/date types and adds field-access classes for TIME_FIELDS. |
| cpp/ql/lib/change-notes/2026-02-06-UncheckedLeapYearAfterModification_Refactor | Adds changelog entry documenting the refactor and alert reduction. |
This PR addresses excessive false positive alerting from
Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql. In separate in-depth auditing, the number of alerts drops from 40,000 down to 2,000 with these changes, with a much higher rate of true positives, though still too high to be considered more than medium precision (~25% false positive rate remaining).This PR is a complete refactor of the original query to address the false positives observed on production code. Some of the lessons learned here could be extrapolated into the LeapYear.qll library, but leaving changes like this for future PRs.