834526 - IPC Channel uses debug-only check for number of FDs in a single message, could overwrite stack

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v1

IPC Channel uses a debug-only check for number of FDs that may be transmitted in a single message. That's currently |FileDescriptorSet::MAX_DESCRIPTORS_PER_MESSAGE| (currently 4 in our code, 5 in chromium tip). It uses this to allocate some stack space, and if we exceed this limit then we will overwrite the stack.

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 706173 [details] [diff] [review]
Patch, v1

Add a note that the return isn't reached.

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 706173 [details] [diff] [review]
Patch, v1

This shouldn't happen in current code, but if content can somehow force the b2g process to hit this path, then we'll overflow a stack buffer in an sg:crit way.  This check prevents that path from being reached.

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4b48effcf696
https://hg.mozilla.org/releases/mozilla-b2g18/rev/fb2bb1320ac7

Comment 4

[Al Billings [:abillings - ex-MoCo]]

Can we get a security rating on this issue?

I assume this only affects trunk and no previous versions?

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

If it were triggered, the vulnerability here would be sec-critical.

However, to the best of our knowledge it can't be triggered in current code.

The buggy code exists in gecko19 and gecko20 but all clients of IPC are fully trusted.  So I don't think we need to uplift the patch there, although there's no risk in doing so.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4b48effcf696