Skip to content

Commit 1f63bee

Browse files
bdrodesbdrodes
committed
Added a FP test case and corrected the leap year logic to no longer flag this case.
1 parent df3fcfc commit 1f63bee

File tree

3 files changed

+51
-8
lines changed

3 files changed

+51
-8
lines changed

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

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -629,27 +629,36 @@ class LeapYearGuardCondition extends GuardCondition {
629629
// form: `(year % 4 == 0) && (year % 100 != 0 || year % 400 == 0)`
630630
// or : `!((year % 4 == 0) && (year % 100 != 0 || year % 400 == 0))`
631631
// Also accepting `((year & 3) == 0) && (year % 100 != 0 || year % 400 == 0)`
632+
// and `(year % 4 == 0) && (year % 100 > 0 || year % 400 == 0)`
632633
this = andExpr and
633634
andExpr.hasOperands(div4Check, orExpr) and
634635
orExpr.hasOperands(div100Check, div400Check) and
635636
(
637+
// year % 4 == 0
636638
exists(RemExpr e |
637639
div4Check.comparesEq(e, 0, true, gv) and
638640
e.getRightOperand().getValue().toInt() = 4 and
639641
yearSinkDiv4 = e.getLeftOperand()
640642
)
641643
or
644+
// year & 3 == 0
642645
exists(BitwiseAndExpr e |
643646
div4Check.comparesEq(e, 0, true, gv) and
644647
e.getRightOperand().getValue().toInt() = 3 and
645648
yearSinkDiv4 = e.getLeftOperand()
646649
)
647650
) and
648651
exists(RemExpr e |
649-
div100Check.comparesEq(e, 0, false, gv) and
652+
(
653+
// year % 100 != 0 or year % 100 > 0
654+
div100Check.comparesEq(e, 0, false, gv)
655+
or
656+
div100Check.comparesLt(e, 1, false, gv)
657+
) and
650658
e.getRightOperand().getValue().toInt() = 100 and
651659
yearSinkDiv100 = e.getLeftOperand()
652660
) and
661+
// year % 400 == 0
653662
exists(RemExpr e |
654663
div400Check.comparesEq(e, 0, true, gv) and
655664
e.getRightOperand().getValue().toInt() = 400 and
@@ -659,29 +668,41 @@ class LeapYearGuardCondition extends GuardCondition {
659668
// Inverted logic case:
660669
// `year % 4 != 0 || (year % 100 == 0 && year % 400 != 0)`
661670
// or `year & 3 != 0 || (year % 100 == 0 && year % 400 != 0)`
671+
// also accepting `year % 4 > 0 || (year % 100 == 0 && year % 400 > 0)`
662672
this = orExpr and
663673
orExpr.hasOperands(div4Check, andExpr) and
664674
andExpr.hasOperands(div100Check, div400Check) and
665675
(
676+
// year % 4 != 0 or year % 4 > 0
666677
exists(RemExpr e |
667-
div4Check.comparesEq(e, 0, false, gv) and
678+
(
679+
div4Check.comparesEq(e, 0, false, gv) or
680+
div4Check.comparesLt(e, 1, false, gv)
681+
) and
668682
e.getRightOperand().getValue().toInt() = 4 and
669683
yearSinkDiv4 = e.getLeftOperand()
670684
)
671685
or
686+
// year & 3 != 0
672687
exists(BitwiseAndExpr e |
673688
div4Check.comparesEq(e, 0, false, gv) and
674689
e.getRightOperand().getValue().toInt() = 3 and
675690
yearSinkDiv4 = e.getLeftOperand()
676691
)
677692
) and
693+
// year % 100 == 0
678694
exists(RemExpr e |
679695
div100Check.comparesEq(e, 0, true, gv) and
680696
e.getRightOperand().getValue().toInt() = 100 and
681697
yearSinkDiv100 = e.getLeftOperand()
682698
) and
699+
// year % 400 != 0 or year % 400 > 0
683700
exists(RemExpr e |
684-
div400Check.comparesEq(e, 0, false, gv) and
701+
(
702+
div400Check.comparesEq(e, 0, false, gv)
703+
or
704+
div400Check.comparesLt(e, 1, false, gv)
705+
) and
685706
e.getRightOperand().getValue().toInt() = 400 and
686707
yearSinkDiv400 = e.getLeftOperand()
687708
)

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
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. |
2424
| 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. |
2525
| 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. |
26+
| test.cpp:1629:2:1629:22 | ... += ... | test.cpp:1629:2:1629:22 | ... += ... | test.cpp:1629:2:1629:22 | ... += ... | Year field has been modified, but no appropriate check for LeapYear was found. |
2727
edges
2828
| test.cpp:813:21:813:40 | ... + ... | test.cpp:813:2:813:40 | ... = ... | provenance | |
2929
| test.cpp:818:13:818:24 | ... + ... | test.cpp:818:2:818:24 | ... = ... | provenance | |
@@ -158,8 +158,10 @@ nodes
158158
| test.cpp:1573:2:1573:22 | ... += ... | semmle.label | ... += ... |
159159
| test.cpp:1584:2:1584:22 | ... += ... | semmle.label | ... += ... |
160160
| test.cpp:1596:2:1596:22 | ... += ... | semmle.label | ... += ... |
161-
| test.cpp:1609:2:1609:22 | ... += ... | semmle.label | ... += ... |
162-
| test.cpp:1644:2:1644:22 | ... += ... | semmle.label | ... += ... |
163-
| test.cpp:1649:2:1649:22 | ... += ... | semmle.label | ... += ... |
164-
| test.cpp:1678:2:1678:22 | ... += ... | semmle.label | ... += ... |
161+
| test.cpp:1608:2:1608:22 | ... += ... | semmle.label | ... += ... |
162+
| test.cpp:1618:2:1618:22 | ... += ... | semmle.label | ... += ... |
163+
| test.cpp:1629:2:1629:22 | ... += ... | semmle.label | ... += ... |
164+
| test.cpp:1664:2:1664:22 | ... += ... | semmle.label | ... += ... |
165+
| test.cpp:1669:2:1669:22 | ... += ... | semmle.label | ... += ... |
166+
| test.cpp:1698:2:1698:22 | ... += ... | semmle.label | ... += ... |
165167
subpaths

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1604,6 +1604,26 @@ void odd_leap_year_check3(tm timeinfo){
16041604
}
16051605
}
16061606

1607+
void odd_leap_year_check4(tm timeinfo){
1608+
timeinfo.tm_year += 1;
1609+
WORD year = timeinfo.tm_year + 1900;
1610+
1611+
if( (year % 4 == 0) && (year % 100 > 0 || (year % 400 == 0)))
1612+
{
1613+
// do something
1614+
}
1615+
}
1616+
1617+
void odd_leap_year_check5(tm timeinfo){
1618+
timeinfo.tm_year += 1;
1619+
WORD year = timeinfo.tm_year + 1900;
1620+
1621+
if( (year % 4 > 0) || (year % 100 == 0 && (year % 400 > 0)))
1622+
{
1623+
// do something
1624+
}
1625+
}
1626+
16071627

16081628
void date_adjusted_through_mkgmtime(tm timeinfo){
16091629
timeinfo.tm_year += 1; // $ SPURIOUS: Alert[cpp/microsoft/public/leap-year/unchecked-after-arithmetic-year-modification]

0 commit comments

Comments
 (0)