1527534 - (CVE-2019-9798) On Android, Gecko always tries to load a library from an all-user-writable path APITRACE_LIB

Status: RESOLVED FIXED

(Core :: Graphics, defect, P1)

Description

[Kelsey Gilbert [:jgilbert]]

https://searchfox.org/mozilla-central/rev/cb7faaf6b4ad2528390186f1ce64618dea71031e/gfx/gl/GLLibraryEGL.h#21

#ifdef ANDROID
// We only need to explicitly dlopen egltrace
// on android as we can use LD_PRELOAD or other tricks
// on other platforms. We look for it in /data/local
// as that's writeable by all users
//
// This should really go in GLLibraryEGL.cpp but we currently reference
// APITRACE_LIB in GLContextProviderEGL.cpp. Further refactoring
// will come in subsequent patches on Bug 732865
#  define APITRACE_LIB "/data/local/tmp/egltrace.so"
#endif

We should not be loading libraries from locations that other apps can write to. It sounds like might be the case here.

This should be opt-in, maybe by pref.

Comment 1

[Daniel Veditz [:dveditz]]

sec-high because it'd be pretty easy for a malicious app (evidence of their existence abounds) to completely MITM anything you do in your browser.

Comment 2

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1527534 - Reuse LoadApitraceLibrary.

Comment 3

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 9045094 [details]
Bug 1527534 - Reuse LoadApitraceLibrary.

Security Approval Request

How easily could an exploit be constructed based on the patch?

Difficult. You have to realize that GLContext's loading of the APITRACE library isn't restricted by the pref, /and also/ have control of a malicious installed app to write a malicious hook library to that location.

Otherwise, this mostly looks like a quality-of-life refactor.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

No

Which older supported branches are affected by this flaw?

all

If not all supported branches, which bug introduced the flaw?

None

Do you have backports for the affected branches?

No

If not, how different, hard to create, and risky will they be?

Trivial

How likely is this patch to cause regressions; how much testing does it need?

Unlikely to regress if tests pass.

Comment 4

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

Comment on attachment 9045094 [details]
Bug 1527534 - Reuse LoadApitraceLibrary.

Sec-approval+. Can we get a beta and ESR60 patch as well?

Comment 5

[Kelsey Gilbert [:jgilbert]]

ESR60 is unaffected because we don't ship ESR on Android.

Comment 6

[Kelsey Gilbert [:jgilbert]]

Applies cleanly to Beta.

Comment 7

[Kelsey Gilbert [:jgilbert]]

Oops, it might not apply cleanly to central once bug 1528396 lands.

Comment 8

[Kelsey Gilbert [:jgilbert]]

What's the landing schedule?

Comment 9

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

(In reply to Jeff Gilbert [:jgilbert] from comment #8)

What's the landing schedule?

Land whenever it is ready at this point.

Comment 10

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 9045094 [details]
Bug 1527534 - Reuse LoadApitraceLibrary.

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

None

User impact if declined

sec-high

Is this code covered by automated tests?

Yes

Has the fix been verified in Nightly?

No

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)

Any breakage will be really obvious on CI.

String changes made/needed

none

Comment 11

[Kelsey Gilbert [:jgilbert]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6f836dba1aedc6226435ddeab17c0e00f876d72d

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6f836dba1aed

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9045094 [details]
Bug 1527534 - Reuse LoadApitraceLibrary.

Protect against possible exploit, let's take this for beta 11.

Comment 14

[Kelsey Gilbert [:jgilbert]]

https://hg.mozilla.org/releases/mozilla-beta/rev/fa6574ad741f461a236e38872f1a405d702841d9