Assert that actor references are passed only over the correct channel

RESOLVED FIXED in Firefox 67

Status

()

enhancement
RESOLVED FIXED
5 months ago
3 months ago

People

(Reporter: jld, Assigned: Alex_Gaynor)

Tracking

unspecified
mozilla67
Points:
---

Firefox Tracking Flags

(firefox67 fixed)

Details

Attachments

(1 attachment)

IPDL allows passing actor references; for example, one side passes a PBrowserChild* as an argument or struct member, and the other side gets the PBrowserParent* for the object on the other end of the actor.

This requires passing the reference over the same channel (== transport == top-level protocol) as the actor being passed, but we don't appear to assert this, and it's a detail that people might not expect: see, for instance, bug 1521113, which is an error that such an assertion could have caught.

(In theory some cases could be statically detected from the IPDL, although for bug 1521113 it's more complicated because the type in question is a union where only one case is uninhabitable. In any case that's a lot more work than a simple dynamic assertion.)

This should be a simple matter of inserting MOZ_ASSERT(aActor->GetIPCChannel() == aVar->GetIPCChannel(), "…") into the right point of the autogenerated IPDLParamTraits::Write methods, for values of “simple” that include dealing with the IPDL compiler AST stuff. I think a debug assertion should be enough for this, so we don't need to worry about the overhead of the MessageChannel lookups. It could even include the actor class name in the assertion message, because it won't take up space in opt builds.

I also vaguely remember during one of the IPC meetings a suggestion about including the actor type (compactly expressible as the message ID base) in the serialized form and checking for mismatches, but I don't remember why that would be useful.

Assignee

Updated

5 months ago
Assignee: nobody → agaynor
Assignee

Updated

5 months ago
Keywords: checkin-needed

Comment 2

5 months ago

Pushed by ncsoregi@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7213808d10e0
don't allow accidentally passing actors via the wrong managing actor in IPDL messages; r=nika

Keywords: checkin-needed

Comment 3

5 months ago
bugherder
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
See Also: → 1540764
You need to log in before you can comment on or make changes to this bug.