Last Comment Bug 334442 - Incorrect use of realloc oom Crash in secmod_ReadPermDB
: Incorrect use of realloc oom Crash in secmod_ReadPermDB
Status: RESOLVED FIXED
[sg:nse] [CID 224]
: coverity, crash, fixed1.8.0.4, fixed1.8.1
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: 3.11
: All Linux
: P1 critical (vote)
: 3.11.1
Assigned To: Alexei Volkov
:
:
Mentors:
http://bonsai.mozilla.org/cvsblame.cg...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-04-18 00:11 PDT by timeless
Modified: 2006-06-10 19:20 PDT (History)
4 users (show)
dveditz: blocking1.9a1+
dveditz: blocking1.8.1+
dveditz: blocking1.8.0.4+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
properly use realloc (3.02 KB, patch)
2006-04-18 00:19 PDT, timeless
nelson: review+
dveditz: approval‑branch‑1.8.1+
dveditz: approval1.8.0.4+
Details | Diff | Splinter Review

Description timeless 2006-04-18 00:11:45 PDT
found by coverity
Comment 1 timeless 2006-04-18 00:17:17 PDT
please see bug 244478 comment 13 for an explanation of why what this code is doing is very very very wrong.
Comment 2 timeless 2006-04-18 00:19:49 PDT
Created attachment 218783 [details] [diff] [review]
properly use realloc
Comment 3 Nelson Bolyard (seldom reads bugmail) 2006-04-18 04:28:36 PDT
Comment on attachment 218783 [details] [diff] [review]
properly use realloc

r=nelson
Comment 4 Daniel Veditz [:dveditz] 2006-04-18 16:29:56 PDT
How does this crash rather than just leak?
Comment 5 Daniel Veditz [:dveditz] 2006-04-18 16:30:21 PDT
And who's going to check in the patch?
Comment 6 Daniel Veditz [:dveditz] 2006-04-18 16:42:45 PDT
Timeless points out the code says "if (!moduleList[0])", not the
"if (moduleList)" my brain saw.
Comment 7 Nelson Bolyard (seldom reads bugmail) 2006-04-18 17:50:49 PDT
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.
Comment 8 Nelson Bolyard (seldom reads bugmail) 2006-04-19 13:36:22 PDT
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.
Comment 9 Alexei Volkov 2006-04-19 15:56:43 PDT
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
Comment 10 Daniel Veditz [:dveditz] 2006-04-20 11:44:41 PDT
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
Comment 11 Nelson Bolyard (seldom reads bugmail) 2006-04-24 10:43:41 PDT
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 Kai Engert (:kaie) (on vacation) 2006-04-24 11:04:53 PDT
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
Comment 13 Nelson Bolyard (seldom reads bugmail) 2006-04-24 11:23:43 PDT
Many thanks, Kai.
Comment 14 Alexei Volkov 2006-04-24 19:26:10 PDT
thank you, Kai!
Comment 15 Bob Clary [:bc:] 2006-05-22 10:41:38 PDT
Alexei, any idea on how to test this?
Comment 16 Daniel Veditz [:dveditz] 2006-05-26 12:49:29 PDT
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.
Comment 17 Nelson Bolyard (seldom reads bugmail) 2006-05-27 01:47:06 PDT
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.
Comment 18 Nelson Bolyard (seldom reads bugmail) 2006-06-10 19:20:44 PDT
CID 224

Note You need to log in before you can comment on or make changes to this bug.