Skip to content

Jb1/ap1 maturity #315

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.

Sign up for GitHub

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

Open
bdrodes wants to merge 53 commits into
base: main
Choose a base branch
from
Open

Jb1/ap1 maturity #315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
aba87c5
Massaging cpp leap year AP1
ropwareJB Dec 18, 2025
e63e19b
Break out query into subcomponents, comments
ropwareJB Dec 23, 2025
7b5163c
init precise version
ropwareJB Dec 23, 2025
7649370
Test case qlref
ropwareJB Dec 23, 2025
c7a6543
Use Bens version + Autoformat
ropwareJB Dec 26, 2025
f6f63cb
Refactoring common class between dataflow
ropwareJB Dec 26, 2025
50e182e
Hashcons definition of exprEq_propertyPermissive
ropwareJB Dec 26, 2025
ca9f66c
Misc. updates. Removed the ignorable function mechanic, and switched …
bdrodes Dec 29, 2025
ec2e5f7
Code Commenting
ropwareJB Dec 30, 2025
fb72602
Autoformat
ropwareJB Dec 30, 2025
c94edcf
Add failing test case
ropwareJB Dec 30, 2025
6d7bd97
Check for leap day
ropwareJB Dec 30, 2025
662119a
Adding a test for setting a year field through a return arg. Misc. tw…
bdrodes Dec 30, 2025
1169f9e
Assignment through out arg is causing too many FPs.
bdrodes Dec 30, 2025
800abae
Fix mislabeled test case
ropwareJB Dec 31, 2025
ec3b350
Misc. tweaks addressing FPs and cases observed during auditing.
bdrodes Jan 12, 2026
9b7f986
Fixed FP issue with ignorable constants, now no longer relying on the…
bdrodes Jan 12, 2026
9b04b42
A leap year check sink can be a ExprCheckLeapYear component.
bdrodes Jan 12, 2026
2f1a850
Check if there is a guard checking for a month that isnt a february v…
ropwareJB Jan 13, 2026
b013012
Misc. updates. Specifically including how constant values are used to…
bdrodes Jan 13, 2026
ce802bd
More ignorable functions, and adding 0 as an ignorable constant.
bdrodes Jan 13, 2026
3282718
Add TIME_FIELDS struct and test case
ropwareJB Jan 15, 2026
938b42a
Removed unused predicates
ropwareJB Jan 15, 2026
4aaad23
"Precise" query overwrites previous version
ropwareJB Jan 15, 2026
8756089
Removing hashcons usages from LeapYear.qll, inefficient and I'm not s…
bdrodes Jan 15, 2026
2e7dba2
Comment cleanup
bdrodes Jan 15, 2026
9446a6b
Updating query alert message.
bdrodes Jan 15, 2026
bc76018
Misc. false positive and false negative updates. as a response to rev…
bdrodes Jan 16, 2026
63639ea
More FP tweaks.
bdrodes Jan 20, 2026
e55ee18
Updating test for UncheckedReturnValueForTImeFunctions. That query ne…
bdrodes Jan 20, 2026
f507239
New additions to the set of TimeConversionFunctions.
bdrodes Jan 20, 2026
e485d98
Created a new leap year check guard condition. Found that the prior d…
bdrodes Jan 21, 2026
8e7ff2d
Removing most dependencies from UncheckedLeapYearAfterYearMOdificatio…
bdrodes Jan 21, 2026
6c103a9
Time conversion results that are checked in a ternary operator condit…
bdrodes Jan 21, 2026
d36a4df
Misc. FP fixes.
bdrodes Jan 23, 2026
f8a2f5c
More FP tweaks.
bdrodes Jan 23, 2026
acdbb8d
Adding FP checks for assignment of a constant safe date regardless of…
bdrodes Jan 23, 2026
4dcb6f1
Additional test cases and comments
bdrodes Jan 28, 2026
8f1d8d4
Merge branch 'main' into jb1/ap1-maturity
bdrodes Jan 28, 2026
65d9a9d
Now using asDefinition to detect assignment to a year field. Misc cle…
bdrodes Jan 28, 2026
2e613ac
Fixed expected results due to a line shift.
bdrodes Jan 28, 2026
a8d5357
Added an additional test case and fixed a false positive. Also noted …
bdrodes Jan 29, 2026
7604938
Added more heuristic ignorable functions.
bdrodes Jan 29, 2026
27edb07
Adding new div 4 check and false positive test cases
bdrodes Jan 29, 2026
93c7e5d
Comment fix.
bdrodes Jan 29, 2026
86dfe52
Adding a fix and test case for a new false positive, also added a fal…
bdrodes Jan 30, 2026
7ff7006
Adding more false positive cases, and fixing prior test cases to be m…
bdrodes Jan 30, 2026
3ba543e
More heuristics for culling non-gregorian year calculations.
bdrodes Jan 30, 2026
e2d3d15
Minor comment/description clean up, and added a test case and support…
bdrodes Jan 30, 2026
df3fcfc
Changing the name of a predicate to be more precise.
bdrodes Jan 30, 2026
2924f2b
Update cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearMod…
bdrodes Jan 30, 2026
afb058c
Update cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearMod…
bdrodes Jan 30, 2026
56e9c1f
Update cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearMod…
bdrodes Jan 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion cpp/ql/lib/semmle/code/cpp/commons/DateTime.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class PackedTimeType extends Type {
}
}

private predicate timeType(string typeName) { typeName = ["_SYSTEMTIME", "SYSTEMTIME", "tm"] }
private predicate timeType(string typeName) { typeName = ["_SYSTEMTIME", "SYSTEMTIME", "tm", "TIME_FIELDS", "PTIME_FIELDS"] }

/**
* A type that is used to represent times and dates in an 'unpacked' form, that is,
Expand Down Expand Up @@ -95,3 +95,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" }
}
134 changes: 72 additions & 62 deletions cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ class CheckForLeapYearOperation extends Expr {
}
}

bindingset[modVal]
Expr moduloCheckEQ_0(EQExpr eq, int modVal) {
exists(RemExpr rem | rem = eq.getLeftOperand() |
result = rem.getLeftOperand() and
Expand All @@ -50,7 +49,6 @@ Expr moduloCheckEQ_0(EQExpr eq, int modVal) {
eq.getRightOperand().getValue().toInt() = 0
}

bindingset[modVal]
Expr moduloCheckNEQ_0(NEExpr neq, int modVal) {
exists(RemExpr rem | rem = neq.getLeftOperand() |
result = rem.getLeftOperand() and
Expand All @@ -59,48 +57,6 @@ Expr moduloCheckNEQ_0(NEExpr neq, int modVal) {
neq.getRightOperand().getValue().toInt() = 0
}

/**
* Returns if the two expressions resolve to the same value, albeit it is a fuzzy attempt.
* SSA is not fit for purpose here as calls break SSA equivalence.
*/
predicate exprEq_propertyPermissive(Expr e1, Expr e2) {
not e1 = e2 and
(
DataFlow::localFlow(DataFlow::exprNode(e1), DataFlow::exprNode(e2))
or
if e1 instanceof ThisExpr and e2 instanceof ThisExpr
then any()
else
/* If it's a direct Access, check that the target is the same. */
if e1 instanceof Access
then e1.(Access).getTarget() = e2.(Access).getTarget()
else
/* If it's a Call, compare qualifiers and only permit no-argument Calls. */
if e1 instanceof Call
then
e1.(Call).getTarget() = e2.(Call).getTarget() and
e1.(Call).getNumberOfArguments() = 0 and
e2.(Call).getNumberOfArguments() = 0 and
if e1.(Call).hasQualifier()
then exprEq_propertyPermissive(e1.(Call).getQualifier(), e2.(Call).getQualifier())
else any()
else
/* If it's a binaryOperation, compare op and recruse */
if e1 instanceof BinaryOperation
then
e1.(BinaryOperation).getOperator() = e2.(BinaryOperation).getOperator() and
exprEq_propertyPermissive(e1.(BinaryOperation).getLeftOperand(),
e2.(BinaryOperation).getLeftOperand()) and
exprEq_propertyPermissive(e1.(BinaryOperation).getRightOperand(),
e2.(BinaryOperation).getRightOperand())
else
// Otherwise fail (and permit the raising of a finding)
if e1 instanceof Literal
then e1.(Literal).getValue() = e2.(Literal).getValue()
else none()
)
}

/**
* An expression that is the subject of a mod-4 check.
* ie `expr % 4 == 0`
Expand Down Expand Up @@ -226,8 +182,7 @@ class ExprCheckCenturyComponent extends LogicalOrExpr {
ExprCheckCenturyComponent() {
exists(ExprCheckCenturyComponentDiv400 exprDiv400, ExprCheckCenturyComponentDiv100 exprDiv100 |
this.getAnOperand() = exprDiv100 and
this.getAnOperand() = exprDiv400 and
exprEq_propertyPermissive(exprDiv100.getYearExpr(), exprDiv400.getYearExpr())
this.getAnOperand() = exprDiv400
Copy link
Collaborator

@ropwareJB ropwareJB Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to check if the expression referenced is the same in each term?

Copy link
Author

@bdrodes bdrodes Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do that. There is a predicate in this class that is used to get the matching term.

Copy link
Author

@bdrodes bdrodes Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just note that the constructor doesn't care if the terms are the same, it is just looking for any check. The user of this then needs to determine if it is correctly used. At least that's the way I had intended, so you look for a check and see if there is a term that is held by all components, if so and it is the one you traced from, then it is likely a valid check.

)
}

Expand All @@ -252,8 +207,7 @@ final class ExprCheckLeapYearFormA extends ExprCheckLeapYear, LogicalAndExpr {
ExprCheckLeapYearFormA() {
exists(Expr e, ExprCheckCenturyComponent centuryCheck |
e = moduloCheckEQ_0(this.getLeftOperand(), 4) and
centuryCheck = this.getAnOperand().getAChild*() and
exprEq_propertyPermissive(e, centuryCheck.getYearExpr())
centuryCheck = this.getAnOperand().getAChild*()
)
}
}
Expand All @@ -268,12 +222,7 @@ final class ExprCheckLeapYearFormB extends ExprCheckLeapYear, LogicalOrExpr {
exists(VariableAccess va1, VariableAccess va2, VariableAccess va3 |
va1 = moduloCheckEQ_0(this.getAnOperand(), 400) and
va2 = moduloCheckNEQ_0(this.getAnOperand().(LogicalAndExpr).getAnOperand(), 100) and
va3 = moduloCheckEQ_0(this.getAnOperand().(LogicalAndExpr).getAnOperand(), 4) and
// The 400-leap year check may be offset by [1900,1970,2000].
exists(Expr va1_subExpr | va1_subExpr = va1.getAChild*() |
exprEq_propertyPermissive(va1_subExpr, va2) and
exprEq_propertyPermissive(va2, va3)
)
va3 = moduloCheckEQ_0(this.getAnOperand().(LogicalAndExpr).getAnOperand(), 4)
)
}
}
Expand Down Expand Up @@ -410,6 +359,50 @@ class StructTmLeapYearFieldAccess extends LeapYearFieldAccess {
}
}

/**
* `stDate.wMonth == 2`
*/
class DateCheckMonthFebruary extends Operation {
DateCheckMonthFebruary() {
this.getOperator() = "==" and
this.getAnOperand() instanceof MonthFieldAccess and
this.getAnOperand().(Literal).getValue() = "2"
}

Expr getDateQualifier() { result = this.getAnOperand().(MonthFieldAccess).getQualifier() }
}

/**
* `stDate.wDay == 29`
*/
class DateCheckDay29 extends Operation {
DateCheckDay29() {
this.getOperator() = "==" and
this.getAnOperand() instanceof DayFieldAccess and
this.getAnOperand().(Literal).getValue() = "29"
}

Expr getDateQualifier() { result = this.getAnOperand().(DayFieldAccess).getQualifier() }
}

/**
* The combination of a February and Day 29 verification
* `stDate.wMonth == 2 && stDate.wDay == 29`
*/
class DateFebruary29Check extends Operation {
DateFebruary29Check() {
this.getOperator() = "&&" and
exists(DateCheckMonthFebruary checkFeb, DateCheckDay29 check29 |
checkFeb = this.getAnOperand() and
check29 = this.getAnOperand()
)
}

Expr getDateQualifier() {
result = this.getAnOperand().(DateCheckMonthFebruary).getDateQualifier()
}
}

/**
* `Function` that includes an operation that is checking for leap year.
*/
Expand All @@ -425,7 +418,7 @@ class ChecksForLeapYearFunctionCall extends FunctionCall {
}

/**
* A `DataFlow` configuraiton for finding a variable access that would flow into
* A `DataFlow` configuration for finding a variable access that would flow into
* a function call that includes an operation to check for leap year.
*/
private module LeapYearCheckFlowConfig implements DataFlow::ConfigSig {
Expand Down Expand Up @@ -535,13 +528,30 @@ class SafeTimeGatheringFunction extends Function {
* This list of APIs should check for the return value to detect problems during the conversion.
*/
class TimeConversionFunction extends Function {
boolean autoLeapYearCorrecting;

TimeConversionFunction() {
this.getQualifiedName() =
[
"FileTimeToSystemTime", "SystemTimeToFileTime", "SystemTimeToTzSpecificLocalTime",
"SystemTimeToTzSpecificLocalTimeEx", "TzSpecificLocalTimeToSystemTime",
"TzSpecificLocalTimeToSystemTimeEx", "RtlLocalTimeToSystemTime",
"RtlTimeToSecondsSince1970", "_mkgmtime"
]
autoLeapYearCorrecting = false and
(
this.getQualifiedName() =
[
"FileTimeToSystemTime", "SystemTimeToFileTime", "SystemTimeToTzSpecificLocalTime",
"SystemTimeToTzSpecificLocalTimeEx", "TzSpecificLocalTimeToSystemTime",
"TzSpecificLocalTimeToSystemTimeEx", "RtlLocalTimeToSystemTime",
"RtlTimeToSecondsSince1970", "_mkgmtime", "SetSystemTime", "SystemTimeToVariantTime",
"VariantTimeToSystemTime", "VarUdateFromDate", "boost::gregorian::date::from_tm"
]
or
this.getQualifiedName().matches("GetDateFormat%")
)
or
// NOTE: mktime will normalize a Feb 29 on a non-leap year to Mar 1 silently,
autoLeapYearCorrecting = true and
this.getQualifiedName() = ["mktime", "_mktime32", "_mktime64"]
}

/**
* Holds if the function is expected to auto convert a bad leap year date.
*/
predicate isAutoLeapYearCorrecting() { autoLeapYearCorrecting = true }
}
Loading