Pickle::ReadBytes[1] does not allocate memory and copies the data out of the serialized object, instead it just returns a pointer into the serialized object and it is up to the caller to do something with that data (usually use memcpy() to copy it to a persistent location). When unserializing mozilla::net::NetAddr[2][3], ReadBytes is not called properly. NetAddr is defined here[4] with the |raw| union element defined as follows: > union NetAddr { > struct { > uint16_t family; > char data[14]; > } raw; > ... > }; |&aResult->raw.data| returns a pointer to the |data| buffer and casts it to a const char **, now when ReadBytes is called, it will deference that pointer pointer and write a pointer, which points to the serialized data, at the destination. Which means that the first four or eight bytes of |data| correspond to a pointer and not the actual raw address data. I file this bug as a security bug out of precaution, since depending on the usage of the NetAddr object, it could lead to info leak. If this is not the case, I don't see why this should be a security bug. [1] https://dxr.mozilla.org/mozilla-central/rev/c5da92c5b4906369dee83629f81d647226ac1038/ipc/chromium/src/base/pickle.cc#468 [2] https://dxr.mozilla.org/mozilla-central/rev/6764bc656c1d146962d53710d734c2ac87c2306f/netwerk/ipc/NeckoMessageUtils.h#111 [3] https://dxr.mozilla.org/mozilla-central/rev/6764bc656c1d146962d53710d734c2ac87c2306f/netwerk/ipc/NeckoMessageUtils.h#125 [4] https://dxr.mozilla.org/mozilla-central/rev/c5da92c5b4906369dee83629f81d647226ac1038/netwerk/dns/DNS.h#89

Is there any reason why an AF_UNSPEC NetAddr would be being passed over IPC, or can that case just be removed? (For that matter, why would an AF_UNSPEC address exist in the first place? And does the data mean anything in that case?) I looked for uses of "raw.data" (just that text, but NetAddr::raw is an anonymous struct, so accesses probably have that syntax) and found only the one in MDNSResponderReply.cpp, copying *to* it from an actual struct sockaddr. So this probably doesn't need to be a private bug, but someone who knows Necko better should make that call.

(In reply to Jed Davis [:jld] from comment #1) > Is there any reason why an AF_UNSPEC NetAddr would be being passed over IPC, > or can that case just be removed? I don't think that can be removed - generally you would set unspec on it before passing to some kind of resolution function and it would come back indicating v4/v6 with the rest filled out.. or something like that. .. this comes from bug 648878 - so asking nick to take ownership .. tedd I want to confirm I understand what you're saying.. instead of return aMsg->ReadBytes(aIter, reinterpret_cast<const char**> &aResult->raw.data), sizeof(aResult->raw.data)); something like the below?: const char *tmp; if (aMsg->Readbytes(aIter, &tmp, sizeof(aResult->raw.data))) { memcpy (&aResult->RawData, tmp, sizeof(aResult->raw.data)); return true; } return false;

:mcmanus, yes exactly, ReadBytes returns a pointer and checks that the given size fits inside the serialized object (to make sure the other end actually supplied enough data).

Must be "old bugs come back to bite me" day. This one doesn't seem too serious - I'm almost certain we never ship around AF_LOCAL sockets (at least, we didn't when I wrote the original patch, that could've changed). I'm not sure we ship AF_UNSPEC around, though I suspect this would've bitten us already if we did.

remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/f8cd81f16a73