When receiving a SerializedStructuredCloneBuffer over the IPC, the unserialization [1] of the object may lead to a use-after-free. When using ReadBytes [2], a pointer into the serialized object is returned, a common practice that I saw throughout the code, was to use that returned pointer and memcpy() the data to a persistent location. However, the above mentioned code, doesn't copy the data, instead the objects |data| pointer is set to point into the serialized message. The memory region |data| points to, may change during the lifespan of the reference (since that memory is used when receiving messages) which could lead to a serious security vulnerability depending on how the data is used. I discussed this issue with :jld over IRC and both of us couldn't really find anything where this would have an impact (I got lost in the code, I don't know to what extend :jld looked into it) So I am filing this bug as a precaution. The code was introduced in Bug 759427. [1] https://dxr.mozilla.org/mozilla-central/rev/b67316254602a63bf4e568198a5c7d3288a9db27/ipc/glue/IPCMessageUtils.h#752 [2] https://dxr.mozilla.org/mozilla-central/rev/b67316254602a63bf4e568198a5c7d3288a9db27/ipc/glue/IPCMessageUtils.h#762

As long as a SerializedStructuredCloneBuffer or ClonedMessageData can't outlive the Message it's borrowing from, which I think is the same as whether it escapes the extent of the Recv* call, we should be okay. But (1) borrowing the message seems to not be the usual semantics of ParamTraits specializations (e.g., we use nsCString in IPC messages, not nsDependentCString), so people might not be expecting that, and (2) I don't think we have any way to detect mistakes here. I just did a static analysis build with a MOZ_STACK_CLASS annotation on SerializedStructuredCloneBuffer (which would cause an error if a type containing a SerializedStructuredCloneBuffer can be heap-allocated, if I understand correctly) and it completed, which suggests we're not doing anything wrong yet, but I don't know how much assurance that gives us.

> I just did a static analysis build with a MOZ_STACK_CLASS annotation on > SerializedStructuredCloneBuffer (which would cause an error if a type I'm happy to leave this MOZ_STACK_CLASS annotation. jed do you have a patch for this or you want me to do it?

Bill: I think MOZ_STACK_CLASS should be enough to keep the borrowed pointer from escaping, since there isn't really a way for someone to declare a SerializedStructuredCloneBuffer in a larger scope than the message handling code; does that make sense? If it does, then there's no use-after-free (yet) and this doesn't need to be a security bug. But if there's something I missed, then we should figure out what (and probably change this patch's commit message).

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d43f2ace8e9c

Comment on attachment 8710086 [details] [diff] [review] bug1240911-sscb-attr-hg0.diff Review of attachment 8710086 [details] [diff] [review]: ----------------------------------------------------------------- This seems like a nice change. I wonder if we could make the data members of this class private and just expose methods where you send in or get out a JS value. That seems safer, but I'm not sure whether it's possible.