The code for Uniscribe-based text shaping in gfxWindowsFonts.cpp has a number of unchecked uses of nsTArray<T>::SetLength() to resize buffers that will then be passed to Uniscribe APIs. If any of these SetLength() calls fail due to lack of memory (unlikely, but conceivable especially with large textruns), we'd be passing invalid buffers to Uniscribe. This is being fixed on trunk as part of bug 502906, but that (more extensive) patch is not intended for the 1.9.2 branch.

This should prevent us passing invalid buffers to Uniscribe.

Comment on attachment 431835 [details] [diff] [review] check array SetLength() calls for allocation failure Looks good.

Comment on attachment 431835 [details] [diff] [review] check array SetLength() calls for allocation failure Requesting approval to land this for 1.9.2.3 - although I don't know of a clear "smoking gun", we have seen issues with pathological pages/scripts that create huge textruns in the past. Allocating several potentially-large arrays for glyph data without checking for failure, and then passing them to Uniscribe, seems like a risk we should not be taking. So I think we want this on 1.9.2 (and earlier branches?), but I didn't see a "wanted1.9.2?" flag. Should it be blocking? That seems rather extreme for something that is mainly a precautionary measure.

Comment on attachment 431835 [details] [diff] [review] check array SetLength() calls for allocation failure Approved for 1.9.2.3, a=dveditz for release-drivers It looks like this is needed on the 1.9.1 branch. Will this patch work?

Pushed to 1.9.2: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/95c0d61183a1

Comment on attachment 431835 [details] [diff] [review] check array SetLength() calls for allocation failure The same patch applies to 1.9.1 also.

Comment on attachment 431835 [details] [diff] [review] check array SetLength() calls for allocation failure Approved for 1.9.1.10, a=dveditz A checking comment along the lines of a simple "unchecked allocations" would have been better than adding the "potential overflow" part.

(In reply to comment #7) > A checking comment along the lines of a simple "unchecked allocations" would > have been better than adding the "potential overflow" part. Yeah, I thought about that right after I pushed it! Sorry. :( I've already modified the comment in my 1.9.1 tree.

Pushed to 1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/1f3da3f9f51e

This doesn't seem to be a scenario that we can exploit on demand. Is there a way for QA to verify this bug as fixed for 1.9.2 or 1.9.1?

It's basically an out-of-memory scenario, so you'd need to make a testcase that fills memory completely, frees a tidbit, then creates enormous text runs. In short, difficult to hit these cases but not impossible.