1114996 - [openh264] ASAN SEGV on unknown address in WelsFree

Status: RESOLVED FIXED

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

Description

[Nils]

Attachment: repro.264

The attached testcase crashes the ASAN build of h264dec of the upcoming openh264 firefox branch (https://github.com/cisco/openh264/tree/v1.3-Firefox36).

The crash occurs while attempting to free an invalid memory address.

This affects the ASM optimised 32-bit build. The non-asm 32 and 64bit builds crash on a read violation, probably because ASAN is bailing early.

ASAN output:

==28962==ERROR: AddressSanitizer: SEGV on unknown address 0xf3f3f3e3 (pc 0x080801bf bp 0x0829ca98 sp 0xffdd0800 T0)
    #0 0x80801be in __asan::Deallocate(void*, unsigned long, __sanitizer::BufferedStackTrace*, __asan::AllocType) /home/bobthebuilder/llvm/llvm/projects/compiler-rt/lib/asan/asan_allocator2.cc:487:3
    #1 0x80800a9 in __asan::asan_free(void*, __sanitizer::BufferedStackTrace*, __asan::AllocType) (/home/nils/264/h264dec-ff-1.3+0x80800a9)
    #2 0x80f23e2 in __interceptor_free /home/bobthebuilder/llvm/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:31:3
    #3 0x81953a6 in WelsFree (/home/nils/264/h264dec-ff-1.3+0x81953a6)
    #4 0x8196cbe in WelsDec::FreePicture(WelsDec::TagPicture*) (/home/nils/264/h264dec-ff-1.3+0x8196cbe)
    #5 0x8127519 in WelsFreeMem (/home/nils/264/h264dec-ff-1.3+0x8127519)
    #6 0x812b014 in WelsEndDecoder (/home/nils/264/h264dec-ff-1.3+0x812b014)
    #7 0x811dbc9 in WelsDec::CWelsDecoder::Uninitialize() (/home/nils/264/h264dec-ff-1.3+0x811dbc9)
    #8 0x811a77b in main (/home/nils/264/h264dec-ff-1.3+0x811a77b)
    #9 0xf7414a82  (/lib/i386-linux-gnu/libc.so.6+0x19a82)
    #10 0x807e69b in _start (/home/nils/264/h264dec-ff-1.3+0x807e69b)

Comment 1

[Daniel Veditz [:dveditz]]

Ethan: does the dependency mean 
 a) "this is already fixed in upstream" (I think that's what Nils is testing, actually) or
 b) I filed an upstream bug for this, or
 c) this _is_ the upstream bug?

In the case of b) can you add the upstream issue link to the bug?

In the case of c) should this bug "block" 1113777 rather than "depend on" it?

Comment 2

[Ethan Hugg [:ehugg]]

I was under the impression that this bug could not be marked as 'resolved/fixed' until a fixed version lands with some version of Firefox.  Bug 1113777 will hopefully do that by the time we land so this bug depends on that one.  If you have another way of linking these that makes more sense to you, feel free to change it around.

We use the issues list of our GitHub repoository for other bugs, but I do not mirror these sec bugs there as it would make them public.  We will link the pull request IDs of the patches here so it can be easily checked which branches have the fix, and the fixes can be commented on in our GitHub repo.

Comment 3

[Bobby Holley (:bholley)]

Ethan - give that GMP stuff is sandboxed, why is this a sec-high for our product?

Comment 4

[Ethan Hugg [:ehugg]]

Yes, GMP is sandboxed.  Also this bug refers to a branch of OpenH264 'v1.3-Firefox36' which is not used yet for Firefox so I doubt this should be sec-high.   I will defer to Randell on how these should be categorized.

Comment 5

[Nils]

Ethan - Correct me if I am wrong. I believe the Windows sandbox is not yet in the stable release so it won't protect the majority of users.

What is the state of sandboxing openh264 on the other platforms (e.g. Mac, Android, Linux)?

Comment 6

[Maire Reavy [:mreavy]]

(In reply to Nils from comment #5)
> Ethan - Correct me if I am wrong. I believe the Windows sandbox is not yet
> in the stable release so it won't protect the majority of users.
> 
> What is the state of sandboxing openh264 on the other platforms (e.g. Mac,
> Android, Linux)?

All desktop platforms (Win, Mac, and Linux) are sandboxed in release.  Windows was actually our first platform to be sandboxed.  We didn't have OpenH264 for Android until 2-3 weeks ago (thanks to Snorp).  So I believe Fx37 will be the first Android release that supports OpenH264 -- assuming it rides the trains to release.  I do not believe OpenH264 on Android is sandboxed. I'm doing a needinfo to Snorp to confirm.

The best person to ask about security ratings for WebRTC and OpenH264 bugs (and why this bug is rated "sec-high") is dveditz; so I'm needinfo'ing him here.

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

Android gives us some sandboxing for free, and right now that's all we'll have without additional work. What it does, according to my understanding:

1) All Android apps run as their own user id, which prevents them from mucking around with other app's data
2) Child processes are stripped of all Android permissions[0]. This means they can't do stuff like read data from the SD card. You also don't have a JVM, which further limits what you can do.

You could defeat 1) with a root exploit, in which case you are totally owned.

Even with 2) in place, a compromised process can read/write the browser's profile data. I think that is probably the most likely type of attack. If we could prevent local file access we'd be in much better shape.

[0] http://developer.android.com/reference/android/Manifest.permission.html

Comment 8

[Bobby Holley (:bholley)]

(In reply to Nils from comment #5)
> Ethan - Correct me if I am wrong. I believe the Windows sandbox is not yet
> in the stable release so it won't protect the majority of users.

You may be thinking of Firefox content process sandboxing, which is a harder problem than openh264 because a firefox process does a lot more complicated interaction with the system than a GMP instance.

Comment 9

[sijchen]

Same as in https://bugzilla.mozilla.org/show_bug.cgi?id=1114990

Comment 10

[Daniel Veditz [:dveditz]]

This is an "otherwise critical" bug bumped down a notch because it's sandboxed. We rate on the generous side because it's not usually worth the time to prove the bogus address we're freeing can't be controlled by an attacker, or wasn't used elsewhere before ASAN detected it. However, if when fixing the bug we learn that it's impossible or unlikely for an attacker to have controlled the address that can affect the rating, but that's hard to do with these openh264 bugs because we usually don't see patches or know what fixed the problem.

The fact that the bug doesn't apply to the shipping version does not affect the rating; the rating should reflect the severity of the problem had it not been found and fixed before release. We use the various status fields to note which versions are affected.

Comment 11

[Bobby Holley (:bholley)]

(In reply to Daniel Veditz [:dveditz] from comment #10)
> This is an "otherwise critical" bug bumped down a notch because it's
> sandboxed.

Given that we treat sec-high and sec-crit with equal urgency, i think this is unhelpful. It's also inaccurate given the descriptions of the severity ratings. IMO the most appropriate would be sec-moderate, since that's the rating we give bugs that require other bugs to be exploitable (and a sandbox escape would be a pretty darn significant "other bug").

From the wiki:

sec-high: Obtain confidential data from other sites the user is visiting or the local machine, or inject data or code into those sites, requiring no more than normal browsing actions. Indefinite DoS of the user's system, requiring OS reinstallation or extensive cleanup. Exploitable web vulnerabilities that can lead to the targeted compromise of a small number of users. 

sec-moderate: Vulnerabilities which can provide an attacker additional information or positioning that could be used in combination with other vulnerabilities. Disclosure of sensitive information that represents a violation of privacy but by itself does not expose the user or organization to immediate risk. The vulnerability combined with another moderate vulnerability could result in an attack of high or critical severity (aka stepping stone)...

This seems pretty clearly the second.

Comment 12

[Daniel Veditz [:dveditz]]

I was thinking of the browser content sandbox where a rogue content process could access data from any site: definitely still sec-high. What data flows through GMP plugins? Does each video get its own instance? Assuming a single shared plugin what meta data about the videos flow through it, that is what could a compromised process learn by seeing what else you watch? And if it learns anything, are there enough whitelisted API calls in the sandbox to ex-filtrate data (such as by requesting the browser to load a URL)?

Comment 13

[Bobby Holley (:bholley)]

See comment 12.

Comment 14

[Randell Jesup [:jesup] (needinfo me)]

(In reply to Daniel Veditz [:dveditz] from comment #12)
> I was thinking of the browser content sandbox where a rogue content process
> could access data from any site: definitely still sec-high. What data flows
> through GMP plugins?

Camera (or canned) video for compression; encoded video for decompression (where most attacks would occur).  (I'm speaking to OpenH264 here; the idea is similar for EME)


> Does each video get its own instance? Assuming a single
> shared plugin what meta data about the videos flow through it, that is what
> could a compromised process learn by seeing what else you watch? And if it
> learns anything, are there enough whitelisted API calls in the sandbox to
> ex-filtrate data (such as by requesting the browser to load a URL)?

Each GMP process is tied to the plugin *and* origin.  As such, a compromised GMP process could only see data collected using the same origin (and plugin).  A GMP process compromised by evilsite.com can't see video being encoded or decoded for goodsite.com.

It has very limited access to anything.  Mostly just the video streams themselves.  They can easily exfiltrate the encoded stream (that's the point for OpenH264), but they only have access to data that the user allowed them to see.

Details of what APIs are available is jld/etc's bailiwick (and slightly OS-dependent), but it's very locked down.

Comment 15

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

So there's a problem: on the small fraction of Linux desktop systems that are too old to support system call filtering (mostly Debian and RedHat/Centos), we allow non-EME media plugins to be run without sandboxing.  See bug 1074561.

Otherwise, SandboxFilterImplGMP::Build() in security/sandbox/linux/SandboxFilter.cpp has the full list, but there shouldn't be anything there which lets it access resources outside of its own process / thread group / address space other than those passed to it as file descriptors.  (This is a more subtle property than may be obvious; e.g., bug 1066750.)

Comment 16

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

[Additional needinfo to the Windows sandboxing owner.]

Comment 17

[Bob Owen (:bobowen)]

The Windows (Chromium) GMP sandbox's settings are nearly as locked down as Chrome's renderer processes.
I believe the Chromium guys treat these as if they were compromised.

The access token could be dropped to USER_LOCKDOWN from USER_RESTRICTED, but it causes a problem with EME that I need to look into.
From speaking to the Chromium people this would be more important on Windows XP as the Untrusted Integrity Level (which came in with Vista) helps on Vista+.

The other settings are process-level mitigation settings, these are are more nice to have (again my understanding from Chromium guys), but I should be enabling some of these soon.
These seem to be more about making it harder to run arbitrary code from within the vulnerable process itself, than preventing the process from accessing system resources.

Comment 18

[Randell Jesup [:jesup] (needinfo me)]

+glandium since he has wide Linux distro knowledge

(In reply to Jed Davis [:jld] from comment #15)
> So there's a problem: on the small fraction of Linux desktop systems that
> are too old to support system call filtering (mostly Debian and
> RedHat/Centos), we allow non-EME media plugins to be run without sandboxing.
> See bug 1074561.

Options are: 
* disable support for OpenH264/EME on such systems, period (safe, perhaps best)
** Could provide a build-time option to run insecure
* leave it as-is (systems without that level of security support don't get it)
* Require manual enabling of OpenH264/EME on such systems
** Via a non-user-friendly pref (i.e. media.openh264.insecure.enabled, similar for EME) - similar to first option
** Via a having the default in about:plugins for OpenH264/EME be 'Disabled' (probably best, but may not explain well to the user the risks of enabling
** Via UI that pops up ala plugin blocking when you try to use OpenH264/EME

Comment 19

[Steven Michaud [:smichaud] (Retired)]

On the Mac, the GMP plugin sandbox is quite securely locked down -- for example there's only very limited access to the file system (read access to small parts of it).  There's also a content process sandbox ... but it currently allows all access (it has an empty ruleset).

For reference, here are the GMP plugin process and content process rulesets:

https://hg.mozilla.org/mozilla-central/annotate/b42615e51c81/security/sandbox/mac/Sandbox.mm#l19

Comment 20

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

(In reply to Randell Jesup [:jesup] from comment #18)
> Options are: 
> * disable support for OpenH264/EME on such systems, period (safe, perhaps best)
> ** Could provide a build-time option to run insecure

Disabling sandboxing support — build-time with --disable-gmp-sandbox or run-time by setting MOZ_DISABLE_GMP_SANDBOX in the environment — will disable sandboxing requirements as well.  (This is what already happens with builds for platforms like FreeBSD or Linux/PPC — or B2G/mips, if that exists.)

But if we want to revisit bug 1074561, maybe that should happen in its own bug?

Comment 21

[Ethan Hugg [:ehugg]]

OpenH264 1.3 has this fix and is now downloaded for Firefox 34+ so marking this as fixed.