Closed Bug 927687 (CVE-2013-5607) Opened 6 years ago Closed 6 years ago

Avoid unsigned integer wrapping in PL_ArenaAllocate


(NSPR :: NSPR, defect, P2)



(firefox25 fixed, firefox26+ verified, firefox27+ verified, firefox28+ verified, firefox-esr17- wontfix, firefox-esr2425+ verified, b2g18 fixed, b2g-v1.1hd fixed, b2g-v1.2 fixed, b2g-v1.3 fixed)

Tracking Status
firefox25 --- fixed
firefox26 + verified
firefox27 + verified
firefox28 + verified
firefox-esr17 - wontfix
firefox-esr24 25+ verified
b2g18 --- fixed
b2g-v1.1hd --- fixed
b2g-v1.2 --- fixed
b2g-v1.3 --- fixed


(Reporter: wtc, Assigned: wtc)



(Keywords: sec-moderate)


(1 file, 1 obsolete file)

Attached patch Patch (obsolete) — Splinter Review
The attached patch for PL_ArenaAllocate ensures that the sums of unsigned integers
don't wrap.

Note that a->base, a->avail, and a->limit are of the PRUword type, an unsigned
integer type large enough to hold a pointer. The patch relies on the invariant
that a->base <= a->limit and a->avail <= a->limit.
Attachment #818175 - Flags: review?(rrelyea)
Rating sec-critical because of related bug 925100.
Keywords: sec-critical
Friendly nudge for Rob's review.
Attached patch Patch v2Splinter Review
I found that the first two changes in my patch were proposed by
Kamil Dudka (or Pascal Cuoq?) in bug 770534 one year ago.

This patch now contains only the third change.
Attachment #818175 - Attachment is obsolete: true
Attachment #818175 - Flags: review?(rrelyea)
Attachment #823689 - Flags: review?(rrelyea)
Comment on attachment 823689 [details] [diff] [review]
Patch v2

r+ rrelyea
Attachment #823689 - Flags: review?(rrelyea) → review+
Comment on attachment 823689 [details] [diff] [review]
Patch v2

Patch checked in:
Attachment #823689 - Flags: checked-in+
Closed: 6 years ago
Resolution: --- → FIXED
Depends on: 770534
Do we need a separate bug here for updating NSPR in m-c, aurora, and beta?
Flags: needinfo?(wtc)
If there is no risk of confusion, we can just use this bug for updating NSPR
in m-c, aurora, and beta.
Flags: needinfo?(wtc)
(In reply to Wan-Teh Chang from comment #7)
> If there is no risk of confusion, we can just use this bug for updating NSPR
> in m-c, aurora, and beta.

Can you prepare that patch or do we need someone else to?
Flags: needinfo?(wtc)
Also, minusing and wontfixing for esr17 since that will be EOL on the next release date.
Blocks: 935959
(In reply to [:lsblakk] from comment #8)

Kai Engert or I will take care of updating NSPR.
Flags: needinfo?(wtc)
No longer blocks: 935959
I guess this needs a separate CVE because technically NSPR is a separate library even though we ship it along with NSS. CVE-2013-5607
Alias: CVE-2013-5607
I think bug 935568 has fixed this by updating NSPR - is that correct? Anything else need doing here?
Flags: needinfo?(wtc)
Matt - this is another one, do you mind tying up the loose ends here before we ship?  A bunch of bugs were fixed by the respin of 25.0 and esr - is this one of them?
Flags: needinfo?(mwobensmith)
I assume that this was updated because of bug 935568. Others can feel free to weigh in, but this tells me that the bug has been verified.
Flags: needinfo?(mwobensmith)
Flags: needinfo?(wtc)
Group: crypto-core-security → core-security
b2g18 (v1.1) is currently on NSPR 4.9.5. The Mercurial history doesn't go back to the 4.9.x days, but AFAICT, this is the only security bug fixed since then? If so, is it worth taking an update to 4.10.2 just for this?
Flags: needinfo?(wtc)
Summary: I recommend taking an update to NSPR 4.10.2, but there is a
complication with the script. Also, it is more important to
update to NSS_3_14_5_RTM.

Ryan: I checked the NSPR 4.9.6, 4.10, 4.10.1, and 4.10.2 release notes.
You are right, only NSPR 4.10.2 has security bug fixes (bug 770534 is
the other one).

In general NSPR updates have a low risk and should be uneventful.
But in NSPR 4.10 the master source repository changed from CVS to
Mercurial. So the Python script we use to import an NSPR tag also needs
to be updated. This is step 2 in

It seems that the update to isn't too complated:

You don't need to commit the update to because it will
also change where NSS is pulled from. NSS_3_14_5_RTM is still in CVS.
Flags: needinfo?(wtc)
FWIW, NSS is in the process of being updated to 3.14.5 in bug 898431. My understanding is that dkeeler has a patch in progress pending testing. If you feel that updating NSPR to version 4.10.2 is worth doing, can you please attach a patch to bug 935568 and request approval-mozilla-b2g18 on it? Thanks :)
Comment on attachment 823689 [details] [diff] [review]
Patch v2

[Triage Comment]: Taking in 18 for sec-moderate
Attachment #823689 - Flags: approval-mozilla-b2g18+
Group: core-security
You need to log in before you can comment on or make changes to this bug.