Maybe I spoke too soon for bug 1613195. This is hit on start-up. Report is from m-c 20200306-8ab81c8e93ad and the latest Valgrind.

Thread 2 Chrome_~dThread:
Syscall param sendmsg(msg.msg_iov[1]) points to uninitialised byte(s)
   at 0x4E4F607: sendmsg (sendmsg.c:28)
   by 0x10B16523: IPC::Channel::ChannelImpl::ProcessOutgoingMessages() (checkouts/gecko/ipc/chromium/src/chrome/common/ipc_channel_posix.cc:687)
   by 0x10B179AA: IPC::Channel::ChannelImpl::Send(IPC::Message*) (checkouts/gecko/ipc/chromium/src/chrome/common/ipc_channel_posix.cc:797)
   by 0x10B52C6A: mozilla::detail::RunnableMethodImpl<IPC::Channel*, bool (IPC::Channel::*)(IPC::Message*), false, (mozilla::RunnableKind)0, IPC::Message*>::Run() (dist/include/nsThreadUtils.h:1158)
   by 0x10B0A353: MessageLoop::DoWork() (checkouts/gecko/ipc/chromium/src/base/message_loop.cc:442)
   by 0x10B0B208: base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) (checkouts/gecko/ipc/chromium/src/base/message_pump_libevent.cc:321)
   by 0x10B09965: MessageLoop::Run() (checkouts/gecko/ipc/chromium/src/base/message_loop.cc:315)
   by 0x10B13412: base::Thread::ThreadMain() (checkouts/gecko/ipc/chromium/src/base/thread.cc:192)
   by 0x10B0FD09: ThreadFunc(void*) (checkouts/gecko/ipc/chromium/src/base/platform_thread_posix.cc:40)
   by 0x4E446DA: start_thread (pthread_create.c:463)
   by 0x5EC888E: clone (clone.S:95)
 Address 0x22982e55 is 37 bytes inside a block of size 4,096 alloc'd
   at 0x4C2FECB: malloc (vg_replace_malloc.c:307)
   by 0x117E3D: moz_xmalloc (checkouts/gecko/memory/mozalloc/mozalloc.cpp:52)
   by 0x10B11D80: mozilla::BufferList<InfallibleAllocPolicy>::AllocateSegment(unsigned long, unsigned long) (dist/include/mozilla/mozalloc.h:146)
   by 0x10B0D755: mozilla::BufferList<InfallibleAllocPolicy>::WriteBytes(char const*, unsigned long) (dist/include/mozilla/BufferList.h:445)
   by 0x10B0DA41: Pickle::WriteBytes(void const*, unsigned int, unsigned int) (checkouts/gecko/ipc/chromium/src/base/pickle.cc:630)
   by 0x10BB48FE: void mozilla::ipc::WriteIPDLParam<nsTArray<mozilla::layers::MatrixMessage> const&>(IPC::Message*, mozilla::ipc::IProtocol*, nsTArray<mozilla::layers::MatrixMessage> const&) (dist/include/mozilla/ipc/IPDLParamTraits.h:212)
   by 0x10BB47C4: mozilla::layers::PAPZParent::SendLayerTransforms(nsTArray<mozilla::layers::MatrixMessage> const&) (PAPZParent.cpp:54)
   by 0x1136C2E7: mozilla::layers::APZCTreeManager::SendSubtreeTransformsToChromeMainThread(mozilla::layers::AsyncPanZoomController const*) (checkouts/gecko/gfx/layers/apz/src/APZCTreeManager.cpp:3597)
   by 0x1135C9ED: void mozilla::layers::APZCTreeManager::UpdateHitTestingTreeImpl<mozilla::layers::LayerMetricsWrapper>(mozilla::layers::LayerMetricsWrapper const&, bool, mozilla::layers::WRRootId, unsigned int) (checkouts/gecko/gfx/layers/apz/src/APZCTreeManager.cpp:670)
   by 0x11371969: mozilla::layers::APZUpdater::UpdateHitTestingTree(mozilla::layers::Layer*, bool, mozilla::layers::LayersId, unsigned int) (checkouts/gecko/gfx/layers/apz/src/APZCTreeManager.cpp:691)
   by 0x1142823B: mozilla::layers::CompositorBridgeParent::ShadowLayersUpdated(mozilla::layers::LayerTransactionParent*, mozilla::layers::TransactionInfo const&, bool) (checkouts/gecko/gfx/layers/ipc/CompositorBridgeParent.cpp:1281)
   by 0x11445FC8: mozilla::layers::LayerTransactionParent::RecvUpdate(mozilla::layers::TransactionInfo const&) (checkouts/gecko/gfx/layers/ipc/LayerTransactionParent.cpp:443)
 Uninitialised value was created by a heap allocation
   at 0x4C2FECB: malloc (vg_replace_malloc.c:307)
   by 0x117E3D: moz_xmalloc (checkouts/gecko/memory/mozalloc/mozalloc.cpp:52)
   by 0x1054B251: nsTArrayInfallibleAllocator::ResultTypeProxy nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyWithMemutils>::EnsureCapacity<nsTArrayInfallibleAllocator>(unsigned long, unsigned long) (dist/include/nsTArray.h:204)
   by 0x1136C675: _ZN7mozilla6layersL11ForEachNodeINS0_15ReverseIteratorEPNS0_18HitTestingTreeNodeEZNS0_15APZCTreeManager39SendSubtreeTransformsToChromeMainThreadEPKNS0_22AsyncPanZoomControllerEE4$_21ZNS5_39SendSubtreeTransformsToChromeMainThreadES8_E4$_22EENS_8EnableIfIXaasr6IsSameIDTclfp0_fp_EEvEE5valuesr6IsSameIDTclfp1_fp_EEvEE5valueEvE4TypeET0_RKT1_RKT2_ (dist/include/nsTArray.h:2456)
   by 0x1136C2D4: mozilla::layers::APZCTreeManager::SendSubtreeTransformsToChromeMainThread(mozilla::layers::AsyncPanZoomController const*) (checkouts/gecko/gfx/layers/apz/src/APZCTreeManager.cpp:3564)
   by 0x1135C9ED: void mozilla::layers::APZCTreeManager::UpdateHitTestingTreeImpl<mozilla::layers::LayerMetricsWrapper>(mozilla::layers::LayerMetricsWrapper const&, bool, mozilla::layers::WRRootId, unsigned int) (checkouts/gecko/gfx/layers/apz/src/APZCTreeManager.cpp:670)
   by 0x11371969: mozilla::layers::APZUpdater::UpdateHitTestingTree(mozilla::layers::Layer*, bool, mozilla::layers::LayersId, unsigned int) (checkouts/gecko/gfx/layers/apz/src/APZCTreeManager.cpp:691)
   by 0x1142823B: mozilla::layers::CompositorBridgeParent::ShadowLayersUpdated(mozilla::layers::LayerTransactionParent*, mozilla::layers::TransactionInfo const&, bool) (checkouts/gecko/gfx/layers/ipc/CompositorBridgeParent.cpp:1281)
   by 0x11445FC8: mozilla::layers::LayerTransactionParent::RecvUpdate(mozilla::layers::TransactionInfo const&) (checkouts/gecko/gfx/layers/ipc/LayerTransactionParent.cpp:443)
   by 0x10CE1289: mozilla::layers::PLayerTransactionParent::OnMessageReceived(IPC::Message const&) (PLayerTransactionParent.cpp:125)
   by 0x10BDCAF9: mozilla::layers::PCompositorManagerParent::OnMessageReceived(IPC::Message const&) (PCompositorManagerParent.cpp:197)
   by 0x10B4A9E6: mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) (checkouts/gecko/ipc/glue/MessageChannel.cpp:2187)

What environment variables/prefs are you running Firefox with when you hit this?

MatrixMessage is serialized using a PlainOldDataSerializer but it contains a mozilla::Maybe which I don't think is POD. At least, it has its own serialization code here. So we probably shouldn't be using PlainOldDataSerializer for MatrixMessage.

Ah, good catch! I looked at the serialization code for Maybe, but didn't consider that there may be things aggregating a Maybe which use something like PlainOldDataSerializer.

SimpleLayerAttributes also contains a Maybe and uses PlainOldDataSerializer, so that could explain bug 1613009 as well.

I'm working on patches for the two bugs. Question to :twsmith is not relevant any more. I was just curious because the stack looks like it should only occur if Firefox were running with the GPU process enabled, which it shouldn't be in a default configuration.

I audited the remaining uses of PlainOldDataSerializer and found that ScrollbarData also uses it here but contains a Maybe. So that one needs fixing too. All the remaining uses were fine.

Given that it's relatively easy to take a struct and turn a field of type X into Maybe<X> without updating the serializer, this seems like a bit of a footgun in general. I left a comment on bug 900042 since that seems to be the thing blocking a better solution.

Comment on attachment 9133171 [details]
Bug 1620719 - Explicitly serialize MatrixMessage fields. r?botond

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Some messages sent over IPC may have uninitialized data but exploiting it would be pretty difficult because the message is sent from the GPU/main process and filling those bytes with attacker-chosen data would involve some degree of control over the GPU/main process.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: Fx 70 and up
• If not all supported branches, which bug introduced the flaw?: Bug 1565525
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: Should be trivial to create. Same as the patch on this bug, but some branches will have one line modified because one field in MatrixMessage wasn't added until later and later got renamed.
• How likely is this patch to cause regressions; how much testing does it need?: Should be pretty low risk, we're just changing the way a structure gets serialized. Basic smoketesting should be sufficient.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #8)

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Some messages sent over IPC may have uninitialized data but exploiting it would be pretty difficult because the message is sent from the GPU/main process and filling those bytes with attacker-chosen data would involve some degree of control over the GPU/main process.

Note that an attacker that figures out this patch is fixing uninitialized data over IPC might also reasonably discover the same problem in a different struct (which is bug 1613009) which might be slightly easier to exploit because it involves messages from the content process.

Comment on attachment 9133171 [details]
Bug 1620719 - Explicitly serialize MatrixMessage fields. r?botond

Botond pointed out sec-moderate bugs don't need approval. So I'm just going to land this and the other things that are blocked on this.

https://hg.mozilla.org/integration/autoland/rev/7935cf13d83fb94f03823dfd3419340fde74cc44

https://hg.mozilla.org/mozilla-central/rev/7935cf13d83f

The patch landed in nightly and beta is affected.
:kats, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Sure, let's uplift it. It's low risk and I'm always a little unsure about security bugs.

Comment on attachment 9133171 [details]
Bug 1620719 - Explicitly serialize MatrixMessage fields. r?botond

Beta/Release Uplift Approval Request

• User impact if declined: Possible security impact if somebody figures out how to exploit this. Seems unlikely.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Pretty small change with respect to serializing a structure over IPC
• String changes made/needed: none

Comment on attachment 9133171 [details]
Bug 1620719 - Explicitly serialize MatrixMessage fields. r?botond

sec fix for 75.0b7

https://hg.mozilla.org/releases/mozilla-beta/rev/f30fcea84225