Closed Bug 1536097 Opened 8 months ago Closed 8 months ago

Unsafe/undefined behavior with static_cast in U2FHIDTokenManager::Register

Categories

(Core :: DOM: Web Authentication, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: Alex_Gaynor, Assigned: Alex_Gaynor)

Details

Attachments

(5 files)

https://searchfox.org/mozilla-central/source/dom/webauthn/U2FHIDTokenManager.cpp#126-128 and https://searchfox.org/mozilla-central/source/dom/webauthn/U2FHIDTokenManager.cpp#116-118 (probably others as well)

These cast from a uint8_t that is provided by the content process over IPC to an enum. This allows a malicious content processes to set a value that is not a valid member of the enum, which leads to undefined behavior.

In these particular cases the behavior looks undefined, and not actually exploitable, but we should avoid this pattern entirely.

https://wiki.mozilla.org/Security/Sandbox/IPCguide#Do_not_cast_integers_to_enums contains a description of this pattern, and the correct fix (using ContigiousEnumSerializer for the enum).

Component: DOM: Device Interfaces → DOM: Web Authentication
Priority: -- → P2
Assignee: nobody → agaynor
Keywords: checkin-needed

Pushed by aiakab@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0aec0a2b5cb6
Part 1 - convert WebAuthnMaybeAuthenticatorAttachment to use a native IPDL maybe and use ParamTraits for deserialization; r=jcj
https://hg.mozilla.org/integration/autoland/rev/fd19320348e2
Part 2 - convert WebAuthnMaybeMakeCredentialExtraInfo to use a native IPDL maybe; r=jcj
https://hg.mozilla.org/integration/autoland/rev/a41f36938436
Part 3 - convert WebAuthnMaybeGetAssertionExtraInfo to use a native IPDL maybe; r=jcj
https://hg.mozilla.org/integration/autoland/rev/e516a5f9e905
Part 4 - convert UserVerificationRequirement to use ParamTraits for deserialization; r=jcj
https://hg.mozilla.org/integration/autoland/rev/6937e95afc2e
Part 5 - convert AttestationConveyancePreference to use ParamTraits for deserialization; r=jcj

Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.