1384858 - IPC: heap-use-after-free [@ mozilla::layers::CompositableParentManager::ReceiveCompositableUpdate]

Status: RESOLVED INCOMPLETE

(Core :: Graphics: Layers, defect, P3)

Description

[Christoph Diehl [:posidron]]

Attachment: callstack.txt

The following crash occurred on mozilla-central revision 20170727-f1693d664f8e

INFO: This is an IPC crash found by the fuzzer faulty - there is no test-case available which leads to an immediate crash for reproduction.

The attached session.txt contains a trace of IPC messages which were sent and received during a session of processing a HTML document with on-going DOM mutations while mutating contents of certain IPC messages at the same time.

[...]
[time: 1501133849639092][29402<-29451] [PLayerTransactionParent] Received  PLayerTransaction::Msg_NewCompositable
[time: 1501133849639147][29451->29402] [PLayerTransactionChild]   Sending  PLayerTransaction::Msg_InitReadLocks
[Faulty] pickle field {size_t} of value: 19 changed to: 18
[...]

Comment 1

[Christoph Diehl [:posidron]]

Attachment: session.txt

Comment 2

[Andrew McCreight [:mccr8]]

I'm not too familiar with this code, but it looks like CompositableHost::mLayer is a weak pointer. I'd guess that some parent code relies on the number of things sent by InitReadLocks to be accurate, and it ditches some data structure once it has gotten that many, but then the child sends another one and we hit the UAF?

Comment 3

[Andrew McCreight [:mccr8]]

Well, maybe sec-high. Without being able to run script in the parent, this seems like it would be difficult to turn into arbitrary code execution. But not impossible.

Comment 4

[Stephanie Ouillon [:arroway] (needinfo me)]

Milan, do you know who could be a good person to take a look at this bug?

Comment 5

[Milan Sreckovic [:milan] (needinfo for best results)]

:dvander in a couple of weeks, unless we think this needs to be acted on now.

Comment 6

[David Anderson [:dvander] - inactive, e-mail if emergency]

How can this be reproduced?

Comment 7

[Panos Astithas (he/him) [:past] (please ni?)]

P3 means fix-optional for 56. Is this affecting 57 as well?

Comment 8

[Christoph Diehl [:posidron]]

(In reply to David Anderson [:dvander] from comment #6)
> How can this be reproduced?

There are no direct steps for reproduction due to the nature of the fuzzer.

*** Possible reproduction scenario:

pip install git+https://github.com/mozillasecurity/fuzzfetch
fuzzfetch -a --fuzzing -n firefox -o /tmp

export FAULTY_PROBABILITY=50000
export FAULTY_LARGE_VALUES=1
export FAULTY_PARENT=1
export FAULTY_ENABLE_LOGGING=1
export FAULTY_PICKLE=1
export MOZ_IPC_MESSAGE_LOG=1

Then launch against a test site: https://html5test.com


(In reply to Panos Astithas [:past] (56 Regression Engineering Owner) (please ni?) from comment #7)
> P3 means fix-optional for 56. Is this affecting 57 as well?

Yes.

Comment 9

[Wennie]

Hi Milan:

I have assigned these security bugs to you to reassign them to appropriate developers in your team to investigate and fix them.

Thanks!

Wennie

Comment 10

[Stephanie Ouillon [:arroway] (needinfo me)]

David, did you have any luck at trying the reproduction scenario Christoph suggested?

Comment 11

[David Anderson [:dvander] - inactive, e-mail if emergency]

Sorry, I'm not going to have time to look at this.

Comment 12

[Frederik Braun [:freddy]]

Christoph, in general it would be possible to add extra logging through Faulty to share what IPC Message has been spoofed and what the original and the fuzzed values are?

Also out of desperation I'm CCing dholbert as one of the few people touching code in gfx/layers/ipc/CompositableTransactionParent.cpp recently

Comment 13

[Christoph Diehl [:posidron]]

Frederik, I made those changes https://bugzilla.mozilla.org/show_bug.cgi?id=777067#c80 some days ago and the patch awaits review. The new IPC reports filed recently have original and mutated messages attached.

Comment 14

[David Bolter [:davidb] (NeedInfo me for attention)]

Are there better logs to share, given comment 13?

Comment 15

[Christoph Diehl [:posidron]]

Sadly not for this version of the fuzzer, at that time we had not good logging support.

Comment 16

[Brad Werth [:bradwerth]]

Nothing left to be done here. Additional logs were added some years ago in Bug 777067. The likely source of NULL dereference noted in comment 2 no longer exists.

Comment 17

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

Since the bug is closed, the stalled keyword is now meaningless.
For more information, please visit auto_nag documentation.