1689415 - load of value 65025, which is not a valid value for type 'mozilla::widget::InputContextAction::Cause'

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Tyson Smith [:tsmith]]

This was uncovered by running faulty to fuzz messages sent to the parent process. I have not seen this in any other situation so maybe this is a WONTFIX and could use a suppression. OTOH this code seems to be trying to prevent/detect invalid values but contains UB. As you can imagine this is hit frequently when fuzzing and there seems to be a couple reports of this issue in this file (perhaps more the fuzzer didn't hit?).

/builds/worker/workspace/obj-build/dist/include/ipc/EnumSerializer.h:100:40: runtime error: load of value 65025, which is not a valid value for type 'mozilla::widget::InputContextAction::Cause'
    #0 0x7ff196b5fa37 in IsLegalValue /builds/worker/workspace/obj-build/dist/include/ipc/EnumSerializer.h:100:40
    #1 0x7ff196b5fa37 in IPC::EnumSerializer<mozilla::widget::InputContextAction::Cause, IPC::ContiguousEnumValidatorInclusive<mozilla::widget::InputContextAction::Cause, (mozilla::widget::InputContextAction::Cause)0, (mozilla::widget::InputContextAction::Cause)7> >::Read(IPC::Message const*, PickleIterator*, mozilla::widget::InputContextAction::Cause*) /builds/worker/workspace/obj-build/dist/include/ipc/EnumSerializer.h:62:17
    #2 0x7ff196b251c8 in ReadParam<mozilla::widget::InputContextAction::Cause> src/ipc/chromium/src/chrome/common/ipc_message_utils.h:125:10
    #3 0x7ff196b251c8 in Read /builds/worker/workspace/obj-build/dist/include/ipc/nsGUIEventIPC.h:924:12
    #4 0x7ff196b251c8 in Read<mozilla::widget::InputContextAction> /builds/worker/workspace/obj-build/dist/include/mozilla/ipc/IPDLParamTraits.h:47:12
    #5 0x7ff196b251c8 in bool mozilla::ipc::ReadIPDLParam<mozilla::widget::InputContextAction>(IPC::Message const*, PickleIterator*, mozilla::ipc::IProtocol*, mozilla::widget::InputContextAction*) /builds/worker/workspace/obj-build/dist/include/mozilla/ipc/IPDLParamTraits.h:72:10
    #6 0x7ff196b1ba4b in mozilla::dom::PBrowserParent::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PBrowserParent.cpp:3694:20
    #7 0x7ff1961a5af6 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PContentParent.cpp:6748:32
    #8 0x7ff195ec97fe in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2153:25
    #9 0x7ff195ec5664 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2077:9
    #10 0x7ff195ec7468 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:1925:3
    #11 0x7ff195ec8088 in mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:1956:13
    #12 0x7ff194b9b6d9 in mozilla::RunnableTask::Run() src/xpcom/threads/TaskController.cpp:472:16
    #13 0x7ff194b97dc1 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:753:26
    #14 0x7ff194b958b7 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:611:15
    #15 0x7ff194b95d0d in mozilla::TaskController::ProcessPendingMTTask(bool) src/xpcom/threads/TaskController.cpp:395:36
    #16 0x7ff194ba3251 in operator() src/xpcom/threads/TaskController.cpp:133:37
    #17 0x7ff194ba3251 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_3>::Run() src/xpcom/threads/nsThreadUtils.h:534:5
    #18 0x7ff194bc32ed in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1171:16
    #19 0x7ff194bcee1c in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:548:10
    #20 0x7ff195ed241f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:87:21
    #21 0x7ff195dc8701 in RunInternal src/ipc/chromium/src/base/message_loop.cc:335:10
    #22 0x7ff195dc8701 in RunHandler src/ipc/chromium/src/base/message_loop.cc:328:3
    #23 0x7ff195dc8701 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:310:3
    #24 0x7ff19cd5ba47 in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
    #25 0x7ff1a08fd4a7 in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:271:30
    #26 0x7ff1a0b24244 in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:5159:22
    #27 0x7ff1a0b25ee5 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:5351:8
    #28 0x7ff1a0b26b33 in XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:5410:21
    #29 0x55761cf789d5 in do_main src/browser/app/nsBrowserApp.cpp:220:22
    #30 0x55761cf789d5 in main src/browser/app/nsBrowserApp.cpp:344:16

/builds/worker/workspace/obj-build/dist/include/ipc/EnumSerializer.h:84:40: runtime error: load of value 7012451, which is not a valid value for type 'mozilla::dom::quota::PersistenceType'
    #0 0x7f57a944c88b in IsLegalValue /builds/worker/workspace/obj-build/dist/include/ipc/EnumSerializer.h:84:40
...

Comment 1

[Simon Giesecke [:sg] [he/him]]

While I created that file, I only moved its contents from ipc/glue/IPCMessageUtils.h.

I gave it a look though, and I think the cast to paramType should be removed at https://searchfox.org/mozilla-central/rev/4f07d49f1c7a823da07e3a231ac87c6435c8fd58/ipc/glue/EnumSerializer.h#62 and IsParamValue should rather accept an uintParamType argument (and the classes provding IsParamValue would need to add some cast towards uintParamType). This should eliminate UB.

Comment 2

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1689415 - Avoid UB when validating enum values in EnumSerializer. r=mccr8

Previously, there was undefined behaviour when validating enum values in
EnumSerializer, which were actually invalid (which was hit during fuzzing, e.g.),
because the invalid integral value was cast to the enum type for comparison.
This patch changes the comparison to cast the valid values to their integral
values instead and compare those.

Comment 3

[Andrew McCreight [:mccr8]]

Interesting find! It does seem like a good idea to fix given that this is dealing with untrusted data.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/57f9d0cd0b33
Avoid UB when validating enum values in EnumSerializer. r=mccr8

Comment 5

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/57f9d0cd0b33