1565374 - (CVE-2019-11749) Fingerprinting resistance against getUserMedia constraints doesn't work

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect)

Description

[Andreas Pehrson [:pehrsons]]

Found through bug 1565317 really, which was caused by bug 1560907.
Prior to bug 1560907 code for reducing a set of constraints looked like this:

  // It looks like {audio: true}, do nothing.
  if (!aConstraint.IsMediaTrackConstraints()) {
    return;
  }

  // Keep mediaSource, ignore all other constraints.
  auto& c = aConstraint.GetAsMediaTrackConstraints();
  MOZ_DIAGNOSTIC_ASSERT(c.mMediaSource.WasPassed());
  nsString mediaSource = c.mMediaSource.Value();
  aConstraint.SetAsMediaTrackConstraints().mMediaSource.Construct(mediaSource);

But since aConstraint is already a MediaTrackConstraints, SetAsMediaTrackConstraints() does not Uninit and reset its values. The reduction is basically a no-op.

Comment 1

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Thanks for catching this!

When we fix this, we should improve the test to catch this.

Two ways to test it:

1. Compare track.getConstraints() against passed-in constraints
2. Check for an impossible constraint (Thanks Andreas!) E.g. {video: {width: {min: 9999}}}

As a side-note: About (1) perhaps we shouldn't make it so easy to detect that fingerprinting anti-measures are present? But that's for a follow-up issue perhaps.

Comment 2

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1565374 - Improve test. r?jib

Comment 3

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1565374 - Reset state. r?jib

Depends on D37783

Comment 4

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Andreas, what do you wanna do about the review in bug 1565317? My review comment is the Construct will fail because the value is already there, but I suspect you already know that. 😉

Comment 5

[Andreas Pehrson [:pehrsons]]

(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #4)

Andreas, what do you wanna do about the review in bug 1565317? My review comment is the Construct will fail because the value is already there, but I suspect you already know that. 😉

Missed that actually. I'll just swap it to use Value() instead of Construct() so it can land. Otherwise the crash stays out there for longer.

Comment 6

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Do we need sec review to land this? I'm not seeing a sec rating. I'd consider this sec-moderate fwiw.

Comment 7

[Daniel Veditz [:dveditz]]

https://wiki.mozilla.org/Security/Bug_Approval_Process

sec-moderate sounds appropriate (in which case you don't need sec-approval)

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/autoland/rev/f3f26e1ed9c0a29f447212d9759d0cf723f4d447
https://hg.mozilla.org/integration/autoland/rev/3b125373365743fbc30bc93ef3ee74d865ce9be8
https://hg.mozilla.org/mozilla-central/rev/f3f26e1ed9c0
https://hg.mozilla.org/mozilla-central/rev/3b1253733657

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Given that this is sec-moderate, I'm thinking we don't need to bother uplifting this to ESR60 given that it's nearing the end of its life cycle. That said, do we want this on Beta and ESR68? If so, please nominate :)

Comment 10

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 9077530 [details]
Bug 1565374 - Reset state. r?jib

Beta/Release Uplift Approval Request

• User impact if declined: Scripts may use probing techniques (e.g. bug 1528056)—that don't incur a user prompt—on the getUserMedia API using constraints to reveal device properties of cameras on the system, adding bits to a fingerprint of the user's system, even on builds with privacy.resistFingerprinting=true.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Simple pref-only clearing of optional passed-in constraints argument.
• String changes made/needed: None

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Are there ESR builds built with privacy.resistFingerprinting=true? If so, then this might be of importance to those builds.
• User impact if declined: Same
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Same
• String or UUID changes made by this patch:

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9077529 [details]
Bug 1565374 - Improve test. r?jib

Fixes a potential privacy leak via the getUserMedia API. Approved for 69.0b8 and 68.1esr (I'm sure our Tor friends will appreciate it since they set the privacy.resistFingerprinting pref).

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Actually, this is going to need a rebased patch for uplift.

Comment 13

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: backport.patch

Backport patch for Beta and ESR.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/fcb210ff98cd
https://hg.mozilla.org/releases/mozilla-beta/rev/162ad0aaccf9

https://hg.mozilla.org/releases/mozilla-esr68/rev/491abbed8480
https://hg.mozilla.org/releases/mozilla-esr68/rev/9fd5fe619ab3