Closed
Bug 811756
Opened 13 years ago
Closed 13 years ago
Unchecked cast from NaN double to int in jsdate.cpp
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla20
People
(Reporter: decoder, Unassigned)
Details
(Keywords: sec-want, Whiteboard: [-fsanitize=float-cast-overflow])
Attachments
(1 file)
|
1.03 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
There is an unchecked conversion from NaN double to int in js/src/jsdate.cpp:
> inline int
> DaysInFebruary(int year) {
...
> static double
> DateFromTime(double t)
> {
> double year = YearFromTime(t);
> double d = DayWithinYear(t, year);
>
> int next;
> if (d <= (next = 30))
> return d + 1;
> int step = next;
> if (d <= (next += DaysInFebruary(year))) // <<<--- year can be NaN here
I believe that the reason is a missing check in the beginning of the method DateFromTime, that verifies that t is indeed finite, just like it is done in MonthFromTime.
However, in addition, DaysInFebruary should probably take a double and not int, since that would be converting to int and immediately back to double (because DaysInFebruary immediately calls another function with the year, which again takes double).
The attached patch fixes both.
Attachment #681512 -
Flags: review?(jwalden+bmo)
Comment 1•13 years ago
|
||
Is there a test?
More importantly, is the year of our Lord NaN a leap year or not??
| Reporter | ||
Comment 2•13 years ago
|
||
The test was this:
date = new Date(NaN).setMonth();
No way to detect this though from inside JS (I think).
Comment 3•13 years ago
|
||
In the year NaN, the month of February will have NaN days.
| Reporter | ||
Comment 4•13 years ago
|
||
Does that make it a leap year? ^_^
Comment 5•13 years ago
|
||
I think that makes it NotAYear.
Comment 6•13 years ago
|
||
Comment on attachment 681512 [details] [diff] [review]
Patch
Review of attachment 681512 [details] [diff] [review]:
-----------------------------------------------------------------
rs=me
Attachment #681512 -
Flags: review?(jwalden+bmo) → review+
| Reporter | ||
Comment 7•13 years ago
|
||
Comment 8•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Updated•12 years ago
|
Whiteboard: [-fsanitize=float-cast-overflow]
You need to log in
before you can comment on or make changes to this bug.
Description
•