1278817 - Insufficient length check when unserializing NPNSString

Status: RESOLVED INVALID

(Core Graveyard :: Plug-ins, defect, P5)

Description

[Julian Hector [:tedd] [:jhector]]

When unserializing a NPNSString, a length check is performed [1] to avoid an overflow when multiplying integers.

This check can be bypassed with a negative |length| because everything is a long (or casted to long) which makes this a signed check, depending on the alloc implementation (such as malloc) the buffer allocation [2] could return a buffer of size 0 or a null pointer.

Either way, the return value is not checked and used here [3] which leads to a memcpy() operation. memcpy() expects an unsigned int as its length parameter and will therefore copy data to the buffer.

With a buffer of size 0, this could lead to memory corruption on the heap, or a crash because of a null pointer dereference.

This seems to only affect Mac, I had some trouble debugging this (for one, because I don't have a mac), so this is only theoretical right now. I would appreciate if someone could help further investigating this.

[1] https://dxr.mozilla.org/mozilla-central/rev/1828937da9493b2cd54862b9c520b2ba5c7db92b/dom/plugins/ipc/PluginMessageUtils.h#463
[2] https://dxr.mozilla.org/mozilla-central/rev/1828937da9493b2cd54862b9c520b2ba5c7db92b/dom/plugins/ipc/PluginMessageUtils.h#467
[3] https://dxr.mozilla.org/mozilla-central/rev/1828937da9493b2cd54862b9c520b2ba5c7db92b/dom/plugins/ipc/PluginMessageUtils.h#469

Comment 1

[Benjamin Smedberg]

This is not a security bug. Plugins that use mac strings already aren't in a sandbox and could do bad things directly. And MakeUnique currently uses new[] which is infallible.