1885245 - Remove PlainOldDataSerializer

Status: RESOLVED FIXED

(Core :: IPC, task, P1)

Description

[Kelsey Gilbert [:jgilbert]]

It's not sound, and especially with ParamTraits_TiedFields, there are nearly-as-easy solutions that are actually sound.

Comment 1

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1885245 - Remove PlainOldDataSerializer.

There's no sufficiently robust way to identify POD types in C++, such
that we could rely on this kind of thing for serialization.

As one example, bool must be carefully handled on deserialize, in case
an attacker wants to exploit the UB of bool with value 2.
Additionally, generally it's not viable to tell whether all the members
of a struct are PODs as well, and we need that level of assurance
recursively!

So we instead lean on e.g. ParamTraits_TiedFields/_IsEnumCase for our
extreme robustness requirements.

Comment 2

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1885245 - static_assert(is_empty<T>) in EmptyStructSerializer.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/267149e1dfed
Remove PlainOldDataSerializer. r=mccr8

Comment 4

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/267149e1dfed

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9414501 [details]
Bug 1885245 - Remove PlainOldDataSerializer.

Carrying over the approval requests from bug 1909019...

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Harder to backport fixes to ESR, and losing out on some defense-in-depth
• User impact if declined:
• Fix Landed on Version: 130
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It's on Beta now with no known regressions.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9414501 [details]
Bug 1885245 - Remove PlainOldDataSerializer.

Approved for 128.2esr.

Comment 7

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/85e397e67f3e