1860860 - [meta] Audit ContentParent IPC code with no coverage

Status: NEW

(Core :: DOM: Content Processes, task)

Description

[Andrew McCreight [:mccr8]]

IPDL Recv methods in high privilege processes, written in C++, are an important boundary in our defense against sandbox escapes.

IPC code that is not tested at all is bad for a few reasons:

• It might not work at all.
• Fuzzing that works by mutating IPC messages it sees while running our test suites will miss it.
• If it is really not used, it is providing zero value, only potential attack surface and removing it is likely trivial.

As a stark reminder of the potential consequences of this kind of code, a Safari zero day fixed in April 2023 involved "a very basic buffer overflow vulnerability in unused IPC support code for a disabled feature".

For these reasons, I decided to start auditing unused code in ContentParent.cpp (focusing on IPC-related code). SearchFox makes the code coverage data easy to look at: if code has no coverage, the leftmost gutter on the page is red. Once you find something, you are already in SearchFox so you can start looking into how the code is used. So, my very basic approach here is to scroll down the SearchFox page and look at for red margins.

There's likely unused IPC code in other files, but PContent is a large protocol that often serves as a dumping ground for messages people want to add, so I think it is a good first target. If this proves to be fruitful, maybe we can have a more automated way to find dead code in Recv methods in high privilege processes.

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

:Gijs just mentioned in https://chat.mozilla.org/#/room/#codecoverage:mozilla.org that we apparently don't have code coverage for mac which is probably a relevant concern for this effort. It certainly was surprising to me! Bug 1399394 apparently is tracking with Bug 1467304 as the remaining dependency per :marco.

Comment 2

[Andrew McCreight [:mccr8]]

Good to know! I did not realize that.

Comment 3

[Andrew McCreight [:mccr8]]

Nothing in here was too scary, so I'll just unhide this.