1240904 - Infoleak when unserializing type NPString

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

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

When unserializing an NPString object, memory is leaked because two pointers are used backwards when calling memcpy[1].

ReadBytes returns a pointer into the serialized object and this pointer is stored in |messageBuffer|, |newBuffer| is the newly created buffer that is also returned as part of the unserialized object [2].

But the memcpy [1] first copies data from |newBuffer| into |messageBuffer|, which effectively overwrites the serialized data (ReadBytes has a bound check so we can't write past the end of the serialized message)

When |newBuffer| is then returned, its memory is still unitialized and contains whatever was previously in memory there.

As far as the impact is concerned, I haven't found any usage of NPString, according to git blame Ben Turner introduced the code way back in 2009, but the code that used it vanished over the years.

Which leaves the question if this is dead code, or if it is planned to use it later on again.

[1] https://dxr.mozilla.org/mozilla-central/rev/b67316254602a63bf4e568198a5c7d3288a9db27/dom/plugins/ipc/PluginMessageUtils.h#435
[2] https://dxr.mozilla.org/mozilla-central/rev/b67316254602a63bf4e568198a5c7d3288a9db27/dom/plugins/ipc/PluginMessageUtils.h#436

Comment 1

[Benjamin Smedberg]

The PPluginScriptableObject Variant type uses nsCString, so I doubt that this code is used. If removing it compiles, r=me to do that.

Comment 2

[Andrew McCreight [:mccr8]]

As bsmedberg said, Firefox seems to compile just fine if ParamTraits<NPString> and ParamTraits<NPVariant> are deleted.

Comment 3

[Andrew McCreight [:mccr8]]

I'm going to go ahead and unhide this because it is dead code. Thanks for finding this issue!

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: Remove ParamTraits for NPString and NPVariant.

r=bsmedberg per comment 1.

Comment 5

[Andrew McCreight [:mccr8]]

try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=44a1aaeb1384

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/34fd055f02a1

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/34fd055f02a1

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/34fd055f02a1