Incorrect use of realloc oom Crash in secmod_ReadPermDB

RESOLVED FIXED in 3.11.1

Status

NSS
Libraries
P1
critical
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: timeless, Assigned: Alexei Volkov)

Tracking

(4 keywords)

3.11
3.11.1
All
Linux
coverity, crash, fixed1.8.0.4, fixed1.8.1
Bug Flags:
blocking1.9a1 +
blocking1.8.1 +
blocking1.8.0.4 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:nse] [CID 224], URL)

Attachments

(1 attachment)

3.02 KB, patch
Nelson Bolyard (seldom reads bugmail)
: review+
Details | Diff | Splinter Review
(Reporter)

Description

12 years ago
found by coverity
(Reporter)

Updated

12 years ago
Group: security
Summary: oom Crash in secmod_ReadPermDB → Incorrect use of realloc oom Crash in secmod_ReadPermDB
(Reporter)

Comment 1

12 years ago
please see bug 244478 comment 13 for an explanation of why what this code is doing is very very very wrong.
(Reporter)

Comment 2

12 years ago
Created attachment 218783 [details] [diff] [review]
properly use realloc
Attachment #218783 - Flags: review?(nelson)
Comment on attachment 218783 [details] [diff] [review]
properly use realloc

r=nelson
Attachment #218783 - Flags: review?(nelson) → review+
How does this crash rather than just leak?
Flags: blocking1.9a1+
Flags: blocking1.8.1+
And who's going to check in the patch?
Flags: blocking1.8.0.3?
Timeless points out the code says "if (!moduleList[0])", not the
"if (moduleList)" my brain saw.
NSS team members will do all checkins.  Want to batch them up, since there 
will apprently be quite a few.  I *expect* (not a promise) that most of 
these will go into 3.11.1 in time for FF 2.0 Beta.
Priority: -- → P2
Target Milestone: --- → 3.11.1
Hardware: PC → All
Alexei, please check in the above reviewed fix on both trunk and 3.11 branch.
In the checkin comment, be sure to mention that the patch is 
contributed by timeless@bemail.org
Thanks.
Assignee: nobody → alexei.volkov.bugs
Priority: P2 → P1
(Assignee)

Comment 9

12 years ago
Check into the tip:
/cvsroot/mozilla/security/nss/lib/softoken/pk11db.c,v  <--  pk11db.c
new revision: 1.36; previous revision: 1.35

Check into the 3.11 branch:
/cvsroot/mozilla/security/nss/lib/softoken/pk11db.c,v  <--  pk11db.c
new revision: 1.35.2.1; previous revision: 1.35
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Flags: blocking1.8.0.3? → blocking1.8.0.3+
Comment on attachment 218783 [details] [diff] [review]
properly use realloc

Please check this into the 1.8.0 and 1.8 branches as well, and add "fixed1.8.1" and "fixed1.8.0.3" keywords when you've done that. Thanks!

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #218783 - Flags: approval1.8.0.3+
Attachment #218783 - Flags: approval-branch-1.8.1+
Kai, do you have trees for 1.8.0.3 and 1.8.1+?
If so, would you be willing to do the checkins of this bug's patch on those
trees?  They're already approved (see previous comment).

Comment 12

12 years ago
done

1.8 branch:
Checking in pk11db.c;
/cvsroot/mozilla/security/nss/lib/softoken/pk11db.c,v  <--  pk11db.c
new revision: 1.32.20.2; previous revision: 1.32.20.1
done

1.8.0 branch:
Checking in pk11db.c;
/cvsroot/mozilla/security/nss/lib/softoken/pk11db.c,v  <--  pk11db.c
new revision: 1.32.30.1; previous revision: 1.32
done
Keywords: fixed1.8.0.3, fixed1.8.1
Many thanks, Kai.
(Assignee)

Comment 14

12 years ago
thank you, Kai!

Comment 15

11 years ago
Alexei, any idea on how to test this?
Maybe I'm missing something but I don't see the security issues here -- it looks like the old code is at worse a leak followed immediately by a null deref crash in the OOM case.
Whiteboard: [sg:nse]
Daniel, feel free to remove the security flag from this bug as you see fit.
It was set by the reporter.  I don't see how OOM crashes are exploitable,
either.
Group: security
CID 224
Whiteboard: [sg:nse] → [sg:nse] [CID 224]
You need to log in before you can comment on or make changes to this bug.