1401339 - (CVE-2017-7836) The pingsender executable dynamically loads libcurl, using dlopen and hardcoded library list. [Mac/Linux]

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[Ezra Caltum]

Attachment: poc.c

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.113 Safari/537.36

Steps to reproduce:

Introduction:
The pingsender executable dynamically loads libcurl, using dlopen and hardcoded library list.
According to the manpages, the order of execution for dlopen will be:
     When path does not contain a slash character (i.e. it is just a leaf name), dlopen() searches the following until it finds a compatible Mach-O file:
     $LD_LIBRARY_PATH, $DYLD_LIBRARY_PATH, current working directory, $DYLD_FALLBACK_LIBRARY_PATH.
Further:
	 Note: If DYLD_FALLBACK_LIBRARY_PATH is not set, dlopen operates as if DYLD_FALLBACK_LIBRARY_PATH was set to $HOME/lib:/usr/local/lib:/usr/lib.
The vulnerable file is located on: https://dxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/pingsender/pingsender_unix_common.cpp
The offending dlopen code dlopen is in line 105
		mLib = dlopen(libname, RTLD_NOW);

And in line 84 through 101:
		 const char* libcurlNames[] = {
		    "libcurl.so",
		    "libcurl.so.4",
		    // Debian gives libcurl a different name when it is built against GnuTLS
		    "libcurl-gnutls.so.4",
		    // Older libcurl if we can't find anything better
		    "libcurl.so.3",
		#ifndef HAVE_64BIT_BUILD
		    // 32-bit versions on 64-bit hosts
		    "/usr/lib32/libcurl.so",
		    "/usr/lib32/libcurl.so.4",
		    "/usr/lib32/libcurl-gnutls.so.4",
		    "/usr/lib32/libcurl.so.3",
		#endif
		    // macOS
		    "libcurl.dylib",
		    "libcurl.4.dylib",
		    "libcurl.3.dylib"
		  };

By writing a malicious dylib, in ~/lib, an attacker can attain local execution privileges.
Further - on every subsequent telemetry request, the code will be executed.

Instruction to reproduce:
Save the C file and compile to generate a dyilib:
gcc -c poc.c
gcc -dynamiclib -current_version 1.0  poc.o  -o libcurl.dylib

Ensure that Enable Firefox Health Report is enabled On Firefox -> Preferences -> Advanced -> Data Choices 
Open and close Firefox.

This is a bad security practice, you can read more about it (although on a Windows world) on:
http://blog.opensecurityresearch.com/2014/01/unsafe-dll-loading-vulnerabilities.html


Actual results:

Code was executed (Calculator was opened)


Expected results:

A list with the known paths should had been provided.

Comment 1

[:Gijs (he/him)]

:gsvelto, can you take a look? Looks like you wrote this.

Comment 2

[Gabriele Svelto [:gsvelto] (PTO)]

Yeah, this is going to be tricky on Linux. On Mac it's easy to hardcode the paths where Apple usually puts libcurl, but on Linux there's a plethora of possible places, some architecture dependent (e.g. on Debian). I'll look into this tomorrow, worst case we'll have a very long list.

Comment 3

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Just checking - this isn't a sandbox escape right? I assume post-level3 (actually level2 even), that the content process would not have the ability to write to ~/lib?

Comment 4

[Ezra Caltum]

(In reply to Paul Theriault [:pauljt] from comment #3)
> Just checking - this isn't a sandbox escape right? I assume post-level3
> (actually level2 even), that the content process would not have the ability
> to write to ~/lib?

I'm not familiar with Firefox sandbox architecture, but from what I read in the documentation:

https://wiki.mozilla.org/Security/Sandbox/Deny_Filesystem_Access

"Some directories are read/write protected, but this will not provide real security until the bulk of the $HOME directory is read/write protected.
On OS X, the Firefox Profile directory is stored within ~/Library/Application Support/Firefox/Profiles/. ~/Library is read/write protected with a few exceptions for some specific subdirectories. Access to $HOME and other areas of the filesystem is not restricted. i.e., the content process can read and write to/from anywhere the OS permits: $HOME and temporary directories. The ~/Library read/write prevention could be bypassed because the rest of the $HOME is read/write accessible. For example, a compromised process could add malicious commands to ~/.login-type files to copy data from ~/Library when a user logs in."

Comment 5

[:Gijs (he/him)]

(In reply to Ezra Caltum from comment #4)
> (In reply to Paul Theriault [:pauljt] from comment #3)
> > Just checking - this isn't a sandbox escape right? I assume post-level3
> > (actually level2 even), that the content process would not have the ability
> > to write to ~/lib?
> 
> I'm not familiar with Firefox sandbox architecture, but from what I read in
> the documentation:
> 
> https://wiki.mozilla.org/Security/Sandbox/Deny_Filesystem_Access
> 
> "Some directories are read/write protected, but this will not provide real
> security until the bulk of the $HOME directory is read/write protected.
> On OS X, the Firefox Profile directory is stored within
> ~/Library/Application Support/Firefox/Profiles/. ~/Library is read/write
> protected with a few exceptions for some specific subdirectories. Access to
> $HOME and other areas of the filesystem is not restricted. i.e., the content
> process can read and write to/from anywhere the OS permits: $HOME and
> temporary directories. The ~/Library read/write prevention could be bypassed
> because the rest of the $HOME is read/write accessible. For example, a
> compromised process could add malicious commands to ~/.login-type files to
> copy data from ~/Library when a user logs in."

Unfortunately that wikipage hasn't been updated since october 2016 ( https://wiki.mozilla.org/index.php?title=Security/Sandbox/Deny_Filesystem_Access&action=history ). Based on https://groups.google.com/forum/#!searchin/mozilla.dev.platform/sandbox|sort:date/mozilla.dev.platform/HJZg_RVZDao/I3I3yYuDAwAJ , I think the situation is better than that, especially for 56 which we're shipping next week, but Paul will have more details.

Comment 6

[Daniel Veditz [:dveditz]]

"By writing a malicious dylib, in ~/lib, an attacker can attain local execution privileges."

If an attacker has the ability to put a malicious dylib in ~/lib they _already_ have local execution privileges. As Paul was getting at, this is mostly interesting as a potential vector for sandbox escapes, and the wiki quote shows we know our current protection is incomplete.

> This is a bad security practice, you can read more about it (although on a Windows world) on:
> http://blog.opensecurityresearch.com/2014/01/unsafe-dll-loading-vulnerabilities.html

The Windows problem is worse. The "Current" directory is often easily writable depending on how you launch an executable, and the original search path looked there _first_. Even the "Safe" version eventually looked there. Only if you set the search path to "" can you mitigate the worst of the problem. Even then, if the attacker can write to the installation directory or any of the Windows directories they can usually inject into your process--but then again they could also just overwrite your program or one of the legitimate Windows .dlls.

Comment 7

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Daniel Veditz [:dveditz] from comment #6)
> The Windows problem is worse. The "Current" directory is often easily
> writable depending on how you launch an executable, and the original search
> path looked there _first_. Even the "Safe" version eventually looked there.
> Only if you set the search path to "" can you mitigate the worst of the
> problem. Even then, if the attacker can write to the installation directory
> or any of the Windows directories they can usually inject into your
> process--but then again they could also just overwrite your program or one
> of the legitimate Windows .dlls.

Note that on Windows the pingsender doesn't load any DLL dynamically. This is Mac/Linux specific.

Comment 8

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PATCH] Look for libcurl under platform-specific paths

Tentative patch, I still have to test it on Mac and a couple more Linux distros before asking for review.

Comment 9

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PATCH v2] Look for libcurl under platform-specific paths

Comment 10

[Gabriele Svelto [:gsvelto] (PTO)]

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1cc544bd0bd448b5d98d370cda11f7ff068ad223

Comment 11

[Alessio Placitelli [:Dexter]]

Comment on attachment 8911359 [details] [diff] [review]
[PATCH v2] Look for libcurl under platform-specific paths

Review of attachment 8911359 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me with the comment below addressed (in case we need to fix it).
Did you check if you can still reproduce with the STR from 0 on at least a machine?

::: toolkit/components/telemetry/pingsender/pingsender_unix_common.cpp
@@ +82,5 @@
>  {
> +  const char* libcurlPaths[] = {
> +    "/usr/lib",
> +#ifdef XP_LINUX
> +    "/usr/lib32",

Do we need to check for |HAVE_64BIT_BUILD| here?

Comment 12

[Ezra Caltum]

(In reply to Daniel Veditz [:dveditz] from comment #6)
> "By writing a malicious dylib, in ~/lib, an attacker can attain local
> execution privileges."
> 
> If an attacker has the ability to put a malicious dylib in ~/lib they
> _already_ have local execution privileges. As Paul was getting at, this is
> mostly interesting as a potential vector for sandbox escapes, and the wiki
> quote shows we know our current protection is incomplete.
> 
> > This is a bad security practice, you can read more about it (although on a Windows world) on:
> > http://blog.opensecurityresearch.com/2014/01/unsafe-dll-loading-vulnerabilities.html
> 
> The Windows problem is worse. The "Current" directory is often easily
> writable depending on how you launch an executable, and the original search
> path looked there _first_. Even the "Safe" version eventually looked there.
> Only if you set the search path to "" can you mitigate the worst of the
> problem. Even then, if the attacker can write to the installation directory
> or any of the Windows directories they can usually inject into your
> process--but then again they could also just overwrite your program or one
> of the legitimate Windows .dlls.

Well, I played a little bit with this, and there are two interesting scenarios in Mac:

a) CWD is the first directory where it's searched. (so potential sandbox escape...)
b) By doing dlopen, you're bypassing Mac gatekeeper (you're not getting the pesky "This binary was downloaded from the internet / was not signed" ) dialog box. 
Although I'm happy that it's being taken into consideration (and worked on it quickly... BIG KUDOS!), I'm not sure about the moderate impact classification (due to the gatekeeper bypass by using Firefox signed and trusted binaries....)

Anyways, thanks for your attention. This is my first time disclosing to the Mozilla Foundation and I'm happily surprised by the time to patch.

Comment 13

[Ezra Caltum]

Actually - I'm not sure about the potential sandbox escape, as I wrote before I'm not familiar with the sandbox, but I'm downloading and building Firefox from source now...

Comment 14

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Alessio Placitelli [:Dexter] from comment #11)
> Looks good to me with the comment below addressed (in case we need to fix
> it).
> Did you check if you can still reproduce with the STR from 0 on at least a
> machine?

I've just tested it, double-checked what the pingsender was doing, and I cannot reproduce anymore.

> ::: toolkit/components/telemetry/pingsender/pingsender_unix_common.cpp
> @@ +82,5 @@
> >  {
> > +  const char* libcurlPaths[] = {
> > +    "/usr/lib",
> > +#ifdef XP_LINUX
> > +    "/usr/lib32",
> 
> Do we need to check for |HAVE_64BIT_BUILD| here?

Not really, dlopen() will pick the right library anyway so I thought it wasn't worth the fuss.

Comment 15

[Gabriele Svelto [:gsvelto] (PTO)]

Pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/ebd3c3b2d64442c2b5eb7ab3e87c4b423311f3f4

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ebd3c3b2d644

Comment 17

[Alessio Placitelli [:Dexter]]

Do we need to uplift this (and bug 1402966) to 57?

Comment 18

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Alessio Placitelli [:Dexter] from comment #17)
> Do we need to uplift this (and bug 1402966) to 57?

Yes, we should.

Comment 19

[Gabriele Svelto [:gsvelto] (PTO)]

Comment on attachment 8911359 [details] [diff] [review]
[PATCH v2] Look for libcurl under platform-specific paths

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1310703
[User impact if declined]: Potential security vulnerability, see comment 0
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: I've manually verified it but it hasn't received formal QA
[Needs manual test from QE? If yes, steps to reproduce]: We can use the STR in comment 0, it shouldn't be reproducible anymore with this patch applied.
[List of other uplifts needed for the feature/fix]: Bug 1402966 will also need to be uplifted if this is uplifted, or we'll break tier 3 platforms
[Is the change risky?]: No
[Why is the change risky/not risky?]: the change affects a tool that lives outside of gecko and which was designed so that failures have no impact on Firefox functionality
[String changes made/needed]: None

Comment 20

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8911359 [details] [diff] [review]
[PATCH v2] Look for libcurl under platform-specific paths

Fix a sec moderate, taking it.

Comment 21

[Gabriele Svelto [:gsvelto] (PTO)]

Uplifted:

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

Comment 22

[Mike Hommey [:glandium]]

> Not really, dlopen() will pick the right library anyway so I thought it wasn't worth the fuss.

Well, it won't pick the right one on Linux !x86 !x86-64.

Comment 23

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Mike Hommey [:glandium] from comment #22)
> Well, it won't pick the right one on Linux !x86 !x86-64.

Yep, I was aware of that but couldn't find a reliable way to do it except for adding the entire list of architectures Debian supports to the list. I suppose we can add more as we need them. If there is an easy way to guess the right directory at runtime we could make it more robust though but I haven't investigated that either.

Comment 24

[Mike Hommey [:glandium]]

I'm inclined to question the need to hardcode paths on linux at all. There's no stupid fallback like DYLD_FALLBACK_LIBRARY_PATH or the current working directory, and if an attacker can make you run pingsender with an altered LD_LIBRARY_PATH, you're already owned.

Comment 25

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Mike Hommey [:glandium] from comment #24)
> I'm inclined to question the need to hardcode paths on linux at all. There's
> no stupid fallback like DYLD_FALLBACK_LIBRARY_PATH or the current working
> directory, and if an attacker can make you run pingsender with an altered
> LD_LIBRARY_PATH, you're already owned.

I tend to agree but I'm no security expert so I'm not sure what are the current best practices as far as dlopen() is concerned.

Comment 26

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

(In reply to Mike Hommey [:glandium] from comment #24)
> I'm inclined to question the need to hardcode paths on linux at all. There's
> no stupid fallback like DYLD_FALLBACK_LIBRARY_PATH or the current working
> directory, and if an attacker can make you run pingsender with an altered
> LD_LIBRARY_PATH, you're already owned.

I tend to agree.

0. If you could get libraries into a malicious directory that dlopen would search, probably you could just shadow something like libc that's DT_NEEDED instead.

1. In particular, it looks like we wouldn't search $ORIGIN in official builds, so the search path should be entirely directories specified in system-level config or in the environment (which, as mentioned above, if that's a problem then you're already owned).

2. The only case I can think of that would cause a sandbox escape as of 57 is if Firefox is run from a subdirectory of /tmp (see also bug 1386404), *and* it's built with $ORIGIN in DT_RUNPATH for some reason.  However, in that case the attacker could just replace part of the Firefox install itself, so this change doesn't help.

Comment 27

[Gabriele Svelto [:gsvelto] (PTO)]

Shall I open a bug to drop the hardcoded paths from Linux then? I haven't done it yet because I wanted to be 100% I wouldn't be regressing anything but I'm happy to limit this to Mac, it makes things a lot easier.

Comment 28

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

(In reply to Gabriele Svelto [:gsvelto] from comment #27)
> Shall I open a bug to drop the hardcoded paths from Linux then? I haven't
> done it yet because I wanted to be 100% I wouldn't be regressing anything
> but I'm happy to limit this to Mac, it makes things a lot easier.

Yes, but I'd suggest having glandium double-check my reasoning in comment #26 and make sure this is safe; he knows the dynamic linker better than I do.

Comment 29

[Mike Hommey [:glandium]]

Comment 26 sounds about right.

Comment 30

[Gabriele Svelto [:gsvelto] (PTO)]

Alright, I'll open a new bug to drop the hard-coded paths from Linux.