1694727 - (CVE-2021-24001) ContentParent::RecvSessionHistoryUpdate should not work outside of automation

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P2)

Description

[Andrew McCreight [:mccr8]]

I noticed that there's a PContent message SessionHistoryUpdate with the comment "This is a temporary way to pass index and length through parent process. Used for testing." It looks like what this does is reflect the session history update being sent from the child process back to other children in the same browsing context group, which may not be in the same process.

This seems potentially bad in the case where a child process gets taken over. I haven't found any particular place where this could cause an out of bounds memory access, but maybe manipulating the session history is enough. It also is probably not good that this session history update information coming from the parent needs to be treated as potentially hostile, which people reading code might not expect.

I'm not exactly sure how this could be addressed. Maybe enough time has passed that this message can be removed, or maybe we can have it be ignored (and not sent) when we're not in a testing environment?

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

That code will go away. It was added to test the new setup for async index and length handling. But I can't see where it could cause any issues.

Comment 2

[Daniel Veditz [:dveditz]]

But I can't see where it could cause any issues.

That depends on what the value is used for, and what it could do if a compromised child passed a crafted value back to the parent.

Comment 3

[Andrew McCreight [:mccr8]]

The processes being sent to are in the same browsing context group, so I guess in the worst case imaginable (say, the message magically allows arbitrary execution) it won't have any real impact on non-Fission, and only make Fission as bad as non-Fission, so I'm not sure what the right rating would be.

Comment 4

[Neha Kochar [:neha]]

Tracking this with the rest of fission sessionHistory work. Let's fix this by Fission M7a.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Bug 1694727, remove the testing only code for history.length handling, r=peterv

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

remove the testing only code for history.length handling, r=peterv
https://hg.mozilla.org/integration/autoland/rev/c47401d2b91a5e59b34aa6a97d0f6995687dfff3
https://hg.mozilla.org/mozilla-central/rev/c47401d2b91a

Comment 7

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: advisory.txt