Closed
Bug 793253
Opened 13 years ago
Closed 13 years ago
Supposedly infallible TArray can fail to allocate a buffer
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
mozilla18
People
(Reporter: bzbarsky, Assigned: bzbarsky)
Details
(Keywords: sec-moderate, Whiteboard: [adv-track-main17+][qa?])
Attachments
(1 file)
|
2.73 KB,
patch
|
justin.lebar+bug
:
review+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
nsTArray_base<Alloc>::EnsureCapacity has this bit:
if ((uint64_t)capacity * elemSize > size_type(-1)/2) {
NS_ERROR("Attempting to allocate excessively large array");
return false;
}
If Alloc is nsTArrayInfallibleAllocator, we should be aborting here, instead of returning false, I believe.
| Assignee | ||
Comment 1•13 years ago
|
||
I'm not convinced about the checkin comment if we want to not make this obvious... suggestions?
Attachment #663499 -
Flags: review?
| Assignee | ||
Updated•13 years ago
|
Assignee: nobody → bzbarsky
Whiteboard: [need review]
Attachment #663499 -
Flags: review? → review?(justin.lebar+bug)
Comment 2•13 years ago
|
||
Comment on attachment 663499 [details] [diff] [review]
Infallible TArrays should really be infallible.
I'm not convinced anyone would have an easy time developing a testcase out of this, and anyway the patch speaks for itself, so I don't feel like we need to obfuscate the commit message, personally.
Attachment #663499 -
Flags: review?(justin.lebar+bug) → review+
| Assignee | ||
Comment 3•13 years ago
|
||
Flags: in-testsuite-
Whiteboard: [need review]
Target Milestone: --- → mozilla18
Comment 4•13 years ago
|
||
Comment 5•13 years ago
|
||
We should probably get this into Firefox 17, but unless we've found a specific case where this gets abused we don't need to over-worry about esr10 and rushing into Firefox 16 at the last minute.
status-firefox-esr10:
--- → wontfix
status-firefox17:
--- → affected
tracking-firefox17:
--- → +
tracking-firefox18:
--- → +
Keywords: sec-moderate
Comment 6•12 years ago
|
||
Let's get this into 17 so it's in the first ESR17 and also nominate for esr10.
Comment 7•12 years ago
|
||
sorry, cancel that esr10 request, i see now that we marked wontfix there.
| Assignee | ||
Comment 8•12 years ago
|
||
Comment on attachment 663499 [details] [diff] [review]
Infallible TArrays should really be infallible.
OK, requesting approval for 17...
[Approval Request Comment]
Bug caused by (feature/regressing bug #): Long-standing bug
User impact if declined: Probably none, but if there is impact it would be a
security bug.
Testing completed (on m-c, etc.): On trunk for a while, and seems OK.
Risk to taking this patch (and alternatives if risky): Main risk is if a caller
was depending on this non-fatal behavior in an OOM situation. Then this
fix would introduce a DoS attack.
String or UUID changes made by this patch: None.
Attachment #663499 -
Flags: approval-mozilla-beta?
Comment 9•12 years ago
|
||
Comment on attachment 663499 [details] [diff] [review]
Infallible TArrays should really be infallible.
Approving for uplift so we get this not only for Beta but for our first ESR17 builds.
Attachment #663499 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 10•12 years ago
|
||
status-firefox-esr17:
--- → fixed
Updated•12 years ago
|
Whiteboard: [adv-track-main17+]
Comment 11•12 years ago
|
||
Is there something QA can do to verify this fix?
Whiteboard: [adv-track-main17+] → [adv-track-main17+][qa?]
Updated•12 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•