Closed Bug 383570 Opened 17 years ago Closed 11 years ago

Many string functions hide OOM

Categories

(Core :: XPCOM, defect)

Product:
Core
Component:
XPCOM
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED DUPLICATE of bug 737164

People

(Reporter: jruderman, Unassigned)

Details

(Keywords: sec-want, Whiteboard: [sg:want P2]) 

Many string-modification functions hide out-of-memory errors.  The string ends up unchanged or truncated, but the function is void, so the caller has no easy way to notice.

If the string happens to be one whose integrity matters for a security check (e.g. a hostname), this could be bad...

Bug 335957 comment 2 makes it sound like this is kind of intentional.
Yes it was, for the "ordinary" case where failed string manipulations are kinda-sorta-ok.

Other than waiting for Moz2 and exceptions, do you have an idea how we could fix this? If we knew all the string manipulations that were security-sensitive we could possibly switch those to use the new OOM-aware BeginWriting signatures, but that sounds like an impossible task.

Or we could alternatively just do a hard-exit whenever we run out of memory (e.g. override malloc and operator new), and use a special "null is ok" allocator if we really have allocations (such as in the cache or imagelib) which are truly OOM-aware.
We have a number of reasonaly OOM-aware allocations, really.  Just existing on OOM would be unfortunate, esp. on mobile devices.  timeless probably knows more about how well we deal with OOM in general...

How hard would it be to hard-exit on string allocation failures only, pending moz2?
I would be interested in the hard-exit whenever we run out of memory.  

In release builds, we could reserve memory.  During allocation failures, the requested memory could be returned from our reserve, and then we can ask subsystems to purge caches, etc.
Whiteboard: [sg:investigate]
Chris Jones may have a plan.
Whiteboard: [sg:investigate] → [sg:want P2]
One fix is to switch our strings to infallible allocators.  I'm not really sure how this might affect strings of content-determined length; by analogy with content-determined pixel buffers e.g., we might want fallibly allocated strings in some circumstances.

bz, do we already limit strings to sane lengths?  If so, what's the general approach?
> bz, do we already limit strings to sane lengths? 

No.
OK.  Actually, that seems better; making these infallible shouldn't regress anything, if the signatures are void anyway.
The consumers that really care SetCapacity and then check the resulting capacity to see whether the size increase succeeded.
Unfortunately, many of the callers (such as the += operator) do that check, so they get incorrect results when they should crash.
We can make fallible and infallible versions of SetCapacity().
That seems reasonable to me.
We did this.
Group: core-security
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WORKSFORME
In bug 526500?
Resolution: WORKSFORME → DUPLICATE
Component: String → XPCOM
You need to log in before you can comment on or make changes to this bug.