Skip to content

Commit 86dfe52

Browse files
committed
Adding a fix and test case for a new false positive, also added a false positive we currently do not have a solution for, marked as SPURIOUS.
1 parent 93c7e5d commit 86dfe52

File tree

3 files changed

+37
-0
lines changed

3 files changed

+37
-0
lines changed

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,11 @@ module OperationToYearAssignmentConfig implements DataFlow::ConfigSig {
364364
)
365365
or
366366
isLeapYearCheckSink(n)
367+
or
368+
// If as the flow progresses, the value holding a dangerous operation result
369+
// is apparently being passed by address to some function, it is more than likely
370+
// intended to be modified, and therefore, the definition is killed.
371+
exists(Call c | c.getAnArgument().(AddressOfExpr).getAnOperand() = n.asIndirectExpr())
367372
}
368373

369374
/** Block flow out of an operation source to get the "closest" operation to the sink */

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@
2121
| test.cpp:1467:2:1467:15 | ... = ... | test.cpp:1464:2:1464:10 | ... += ... | test.cpp:1467:2:1467:15 | ... = ... | Year field has been modified, but no appropriate check for LeapYear was found. |
2222
| test.cpp:1497:2:1497:22 | ... += ... | test.cpp:1497:2:1497:22 | ... += ... | test.cpp:1497:2:1497:22 | ... += ... | Year field has been modified, but no appropriate check for LeapYear was found. |
2323
| test.cpp:1505:2:1505:22 | ... += ... | test.cpp:1505:2:1505:22 | ... += ... | test.cpp:1505:2:1505:22 | ... += ... | Year field has been modified, but no appropriate check for LeapYear was found. |
24+
| test.cpp:1584:2:1584:22 | ... += ... | test.cpp:1584:2:1584:22 | ... += ... | test.cpp:1584:2:1584:22 | ... += ... | Year field has been modified, but no appropriate check for LeapYear was found. |
25+
| test.cpp:1596:2:1596:22 | ... += ... | test.cpp:1596:2:1596:22 | ... += ... | test.cpp:1596:2:1596:22 | ... += ... | Year field has been modified, but no appropriate check for LeapYear was found. |
26+
| test.cpp:1609:2:1609:22 | ... += ... | test.cpp:1609:2:1609:22 | ... += ... | test.cpp:1609:2:1609:22 | ... += ... | Year field has been modified, but no appropriate check for LeapYear was found. |
2427
edges
2528
| test.cpp:813:21:813:40 | ... + ... | test.cpp:813:2:813:40 | ... = ... | provenance | |
2629
| test.cpp:818:13:818:24 | ... + ... | test.cpp:818:2:818:24 | ... = ... | provenance | |
@@ -152,4 +155,8 @@ nodes
152155
| test.cpp:1505:2:1505:22 | ... += ... | semmle.label | ... += ... |
153156
| test.cpp:1551:2:1551:22 | ... += ... | semmle.label | ... += ... |
154157
| test.cpp:1562:2:1562:22 | ... += ... | semmle.label | ... += ... |
158+
| test.cpp:1573:2:1573:22 | ... += ... | semmle.label | ... += ... |
159+
| test.cpp:1584:2:1584:22 | ... += ... | semmle.label | ... += ... |
160+
| test.cpp:1596:2:1596:22 | ... += ... | semmle.label | ... += ... |
161+
| test.cpp:1609:2:1609:22 | ... += ... | semmle.label | ... += ... |
155162
subpaths

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1605,3 +1605,28 @@ void odd_leap_year_check3(tm timeinfo){
16051605
}
16061606

16071607

1608+
void date_adjusted_through_mkgmtime(tm timeinfo){
1609+
timeinfo.tm_year += 1; // $ SPURIOUS: Alert[cpp/microsoft/public/leap-year/unchecked-after-arithmetic-year-modification]
1610+
1611+
// Using an odd sytle of checking divisible by 4 presumably as an optimization trick
1612+
// but also check unrelated conditions on the year as an optimization to rule out irrelevant years
1613+
// for gregorian leap years
1614+
if(timeinfo.tm_mon == 2 && (timeinfo.tm_year % 4) == 0 && (timeinfo.tm_year <= 1582 || timeinfo.tm_year % 100 != 0 || timeinfo.tm_year % 400 == 0))
1615+
{
1616+
// do something
1617+
}
1618+
}
1619+
1620+
bool data_killer(WORD *d){
1621+
(*d) = 1;
1622+
return true;
1623+
}
1624+
1625+
void interproc_data_killer1(tm timeinfo, WORD delta){
1626+
WORD year = delta + 1;
1627+
1628+
if(data_killer(&year)){
1629+
timeinfo.tm_year = year;
1630+
}
1631+
}
1632+

0 commit comments

Comments
 (0)