1565996 - [Linux] relative paths in linker includes not handled correctly

Status: VERIFIED FIXED

(Core :: Security: Process Sandboxing, defect, P1)

Description

[Artem S. Tashkinov]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

It turns out Firefox on Linux cannot open shared libraries in /usr/local/(lib|lib64) on Linux which makes it impossible to use it with manually compiled/installed FFmpeg library. See bug 1516178 for more details.

I would like to propose that whitelist these two locations in order to avoid this issue.

Normally they are both owned by the root user and it's very unlikely that's going to affect Firefox security.

Comment 1

[Artem S. Tashkinov]

I presume this issue affects all Unix'es (Linux, FreeBSD, OpenBSD).

Comment 2

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

gcp: Shouldn't the ld.so.conf parser have taken care of this?

Comment 3

[Gian-Carlo Pascutto [:gcp]]

Yes:
https://searchfox.org/mozilla-central/source/security/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp#189

Could you attach ld.so.conf and any relevant include files?

Comment 4

[Artem S. Tashkinov]

$ cat /etc/ld.so.conf 
include ld.so.conf.d/*.conf

$ cat /etc/ld.so.conf.d/local-x86_64.conf 
/usr/local/lib64

Comment 5

[Artem S. Tashkinov]

Looks like you haven't taken into account the last 10 years of Linux development.

Most Linux distros have switched to /etc/ld.so.conf.d.

Comment 6

[Gian-Carlo Pascutto [:gcp]]

Our code already handles the include, so it's still not clear why this isn't working.

Comment 7

[Gian-Carlo Pascutto [:gcp]]

For comparison, this is my own system, which (of course) also uses ld.so.conf.d files:
Sandbox: policy for /dev/nvidia-uvm-tools: 1 -> 7
Sandbox: policy for /dev/nvidia-uvm: 1 -> 7
Sandbox: policy for /dev/nvidia-modeset: 1 -> 7
Sandbox: policy for /dev/nvidia0: 1 -> 7
Sandbox: policy for /dev/nvidiactl: 1 -> 7
...
Sandbox: policy for /usr/local/lib/: 1 -> 35
Sandbox: policy for /usr/local/lib: 1 -> 3
Sandbox: policy for /lib/x86_64-linux-gnu/: 1 -> 35
Sandbox: policy for /lib/x86_64-linux-gnu: 1 -> 3
Sandbox: policy for /usr/lib/x86_64-linux-gnu/: 1 -> 35
Sandbox: policy for /usr/lib/x86_64-linux-gnu: 1 -> 3
...

Those are being picked up from almost exactly the same config as you have (and which is extremely standard anyway...):

/etc/ld.so.conf:
include /etc/ld.so.conf.d/*.conf

/etc/ld.so.conf.d/libc.conf:
# libc default configuration
/usr/local/lib

/etc/ld.so.conf.d/x86_64-linux-gnu.conf:
# Multiarch support
/usr/local/lib/x86_64-linux-gnu
/lib/x86_64-linux-gnu
/usr/lib/x86_64-linux-gnu

(IIRC there's no entry for /usr/local/lib/x86_64-linux-gnu in the log because /usr/local/lib/ supersedes it)

Comment 8

[Gian-Carlo Pascutto [:gcp]]

Okay, one difference I notice is that the path in the ld.so.conf include is relative, not absolute. We allow relative paths in the include, but I wonder if the glob() call is going wrong because it's in the wrong cwd for the relative path.

It seems Fedora ships the ld.so.conf like that, so it'd be surprising this wasn't caught earlier.

Comment 9

[Gian-Carlo Pascutto [:gcp]]

Confirmed that this is the problem.

Comment 10

[Gian-Carlo Pascutto [:gcp]]

Attachment: Bug 1565996 - Handle relative paths in linker config parsing. r?jld

Comment 11

[Artem S. Tashkinov]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #10)

Wow, great many thanks for a quick fix! It's strange no one has filed this bug report earlier.

Comment 12

[Gian-Carlo Pascutto [:gcp]]

jld is already tagged for the review.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/73935c7cf465
Handle relative paths in linker config parsing. r=jld

Comment 14

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/73935c7cf465

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta approval when you get a chance. We may want this on ESR68 also, but maybe after it bakes for a bit first.

Comment 16

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 9079277 [details]
Bug 1565996 - Handle relative paths in linker config parsing. r?jld

Beta/Release Uplift Approval Request

• User impact if declined: Some libraries not picked up in the content process on certain distros. Affects for example video decoding on Fedora if the user updates ffmpeg manually.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• 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): Worst breakage probably puts other Linux distros on the level of Fedora, which this fixes. Gain is somewhat hazy; we did this work to solve long-tail compatibility issues on Linux, so any bugs this fixes are probably not super visible or obvious.
• String changes made/needed:

Comment 17

[Gian-Carlo Pascutto [:gcp]]

Artem, is this working for you in the current Nightly?

Comment 18

[Artem S. Tashkinov]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #17)

Artem, is this working for you in the current Nightly?

Yes, it is, I've just checked the latest nightly. Thanks again!

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9079277 [details]
Bug 1565996 - Handle relative paths in linker config parsing. r?jld

Fixes a Linux-only bug which can cause some libraries to not be properly detected with some distros. Approved for 69.0b9.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

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

Comment 21

[Gian-Carlo Pascutto [:gcp]]

Didn't intend this to require manual QE as it's tricky to set up a case and reporter confirmed the fix.

Comment 22

[Cristian Fogel [:cfogel]]

Marking bug as verified based on comment 18.

@Artem, if you have time mind checking with 69.0b10 as well, once it becomes available?

Comment 23

[Artem S. Tashkinov]

(In reply to Cristian Fogel, QA [:cfogel] from comment #22)

Marking bug as verified based on comment 18.

@Artem, if you have time mind checking with 69.0b10 as well, once it becomes available?

Confirming fixed.

Comment 24

[Cristian Fogel [:cfogel]]

Awesome!
Thank you for the help.