Closed
Bug 1008792
Opened 10 years ago
Closed 9 years ago
Null pointer deref on out of memory in nr_socket_buffered_stun_create
Categories
(Core :: WebRTC: Networking, defect)
Tracking
()
RESOLVED
FIXED
mozilla43
Tracking | Status | |
---|---|---|
firefox43 | --- | fixed |
backlog | parking-lot |
People
(Reporter: ekr, Assigned: mjf)
Details
Attachments
(1 file)
if (!(sock = RCALLOC(sizeof(nr_socket_buffered_stun)))) ABORT(R_NO_MEMORY); sock->inner = inner; ... if (_status) { void *sock_v = sock; sock->inner = 0; /* Give up ownership so we don't destroy */ nr_socket_buffered_stun_destroy(&sock_v); } We need if (sock) around sock->inner
Reporter | ||
Comment 1•10 years ago
|
||
Credit to Sylvestre Ledru for static analysis that caught this.
Comment 2•10 years ago
|
||
This is a potential null-deref in OOM, that doesn't appear to be a sec issue to me.
Flags: needinfo?(abillings)
Updated•10 years ago
|
Group: core-security
Updated•10 years ago
|
Flags: needinfo?(abillings)
Updated•9 years ago
|
backlog: --- → parking-lot
Assignee | ||
Comment 3•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e04d846dcfd0
Assignee | ||
Comment 4•9 years ago
|
||
Bug 1008792: Check for valid pointer before using.
Attachment #8655089 -
Flags: review?(docfaraday)
Comment 5•9 years ago
|
||
Comment on attachment 8655089 [details] MozReview Request: Bug 1008792: Check for valid pointer before using. https://reviewboard.mozilla.org/r/17815/#review15907
Attachment #8655089 -
Flags: review?(docfaraday) → review+
Updated•9 years ago
|
Assignee: nobody → mfroman
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Reporter | ||
Comment 6•9 years ago
|
||
https://reviewboard.mozilla.org/r/17815/#review16021 ::: media/mtransport/third_party/nICEr/src/stun/nr_socket_buffered_stun.c:168 (Diff revision 1) > if (_status) { > void *sock_v = sock; > + if (sock) { > - sock->inner = 0; /* Give up ownership so we don't destroy */ > + sock->inner = 0; /* Give up ownership so we don't destroy */ > + } > nr_socket_buffered_stun_destroy(&sock_v); > } This is an odd change. Why are you calling nr_socket_buffered_stun_destroy() on a pointer you know points to NULL? It's safe, but silly. You should put the entire destructor clause inside the if (sock).
Reporter | ||
Comment 7•9 years ago
|
||
removed checkin-needed because of c6
Flags: needinfo?(mfroman)
Keywords: checkin-needed
Assignee | ||
Updated•9 years ago
|
Attachment #8655089 -
Flags: review+ → review?(docfaraday)
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8655089 [details] MozReview Request: Bug 1008792: Check for valid pointer before using. Bug 1008792: Check for valid pointer before using.
Comment 9•9 years ago
|
||
Comment on attachment 8655089 [details] MozReview Request: Bug 1008792: Check for valid pointer before using. https://reviewboard.mozilla.org/r/17815/#review16033
Attachment #8655089 -
Flags: review?(docfaraday) → review+
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(mfroman)
Keywords: checkin-needed
Comment 10•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b150362b0f4
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7b150362b0f4
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in
before you can comment on or make changes to this bug.
Description
•