Session cookies are using an unsafe expiry max value

RESOLVED FIXED in Firefox 58



2 years ago
2 years ago


(Reporter: whimboo, Assigned: nicole_byer, Mentored)



57 Branch
Firefox 58

Firefox Tracking Flags

(firefox58 fixed)



(1 attachment, 2 obsolete attachments)

Based on the discussion on bug 1408962 comment 8:

(In reply to Tim Taubert [:ttaubert] from comment #8)
> (In reply to Henrik Skupin (:whimboo) from comment #4)
> > Interesting fact Peter! Here is the exact description:
> >
> > Global_Objects/Number/MAX_SAFE_INTEGER
> > 
> > Which makes me wonder why we make use of the following line of code:
> >
> > sessionstore/SessionCookies.jsm#20-21
> > 
> > Tim, you added that code via bug 903398. Can you please have a look if we
> > currently have faulty behavior here?
> Looks like we should probably switch to Number.MAX_SAFE_INTEGER for that
> const. I copied that line from the cookie code in sessionstore that has been
> around since Firefox 2. I'm guessing that the author meant to actually use
> 2^53.

Mike, can someone please get this fixed? Not sure how critical that is, but maybe it might be a good mentored bug?
Flags: needinfo?(mdeboer)
OK, sure. I also read "[...]the integer 2^53 + 1 can't be directly represented in IEEE-754 but instead rounds to 2^53 under round-to-nearest and round-to-zero rounding."[1]; so I think when this value is normalized, it'll also be rounded to 2^53 using that same principle. Is there still an issue, then?

Regardless, I think it'd be best to use `Number.MAX_SAFE_INTEGER` of course.
Mentor: mdeboer
Flags: needinfo?(mdeboer)
Keywords: good-first-bug

Comment 2

2 years ago
Hi, I’m a recent CS graduate and would like to start contributing. Would I be able to take care of this?
Nicole Byer
Hi Nicole, thanks for reaching out and wanting to help out!

You can follow the steps outlined in this document: and get a working build of Firefox. This time you'll be working in frontend Javascript code, so you can use the faster artifact mode to build Firefox.

Then you'll want to open the file browser/components/sessionstore/SessionCookies.jsm in your favorite code editor and change the lines at

Specifically, you'll need to change the Max.pow() statement to use Number.MAX_SAFE_INTEGER instead.

If you've got any further questions, please feel free to reach out to me (or others) here or on IRC (channels #introduction and #developers).
Priority: -- → P3

Comment 4

2 years ago
Hi Mike,
I got a working build of Firefox and changed the line of code (I'll attach the file). What is the next step? Do I need to test this?
Thanks for your help!

Comment 5

2 years ago
Posted patch SessionCookies.jsm (obsolete) — Splinter Review
Lines 20-21 changed to use Number.MAX_SAFE_INTEGER
Hi Nicole, great that you've been able to get a build running! Next step is to submit a patch for review. You can read all about that here:

This is step 4 in the Contributing tutorial at

Comment 7

2 years ago
Ok, great! Are you the reviewer? (I also see ttaubert when I run hg annotate). Thanks!
Yeah, I'm the module owner, so I can review your patch.

Comment 9

2 years ago
Posted patch SessionCookies.jsm (obsolete) — Splinter Review
Changed lines 20-21 to use Number.MAX_SAFE_INTEGER
Attachment #8921457 - Attachment is obsolete: true
Attachment #8921478 - Flags: review?(mdeboer)
Comment on attachment 8921478 [details] [diff] [review]

Nicole, the file you attached is not a patch, but the whole SessionCookies.jsm file, which is something we can't really review.
We always review sets of changes (i.e. a patch), which is quite a bit smaller and doesn't require you to point out what you've changed and where you changed something. Instead you can focus on explaining why you made the change in a commit message.

I really recommend you to give a thorough read-through to see how you can generate a patch.
Attachment #8921478 - Flags: review?(mdeboer)

Comment 11

2 years ago
Sorry about that; I hope this is more along the right lines!
Attachment #8921478 - Attachment is obsolete: true
Attachment #8921544 - Flags: review?(mdeboer)
Comment on attachment 8921544 [details] [diff] [review]
Proposed patch

Review of attachment 8921544 [details] [diff] [review]:

Perfect! Thanks for this :)
Attachment #8921544 - Flags: review?(mdeboer) → review+
Assignee: nobody → nicole_byer

Comment 13

2 years ago
Great, thanks for your help!

Comment 14

2 years ago
Pushed by
Changed use of 2^62 to  Number.MAX_SAFE_INTEGER (represents 2^53 - 1) to avoid faulty behavior. r=mikedeboer
Keywords: checkin-needed
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
You need to log in before you can comment on or make changes to this bug.