Closed Bug 1565996 Opened 9 months ago Closed 8 months ago

[Linux] relative paths in linker includes not handled correctly

Categories

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

68 Branch
All
Linux
defect

Tracking

()

VERIFIED FIXED
mozilla70
Tracking Status
firefox-esr60 --- wontfix
firefox-esr68 --- wontfix
firefox68 --- wontfix
firefox69 --- verified
firefox70 --- verified

People

(Reporter: aros, Assigned: gcp)

References

Details

Attachments

(1 file)

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.

Blocks: 1516178
Component: Untriaged → Security: Process Sandboxing
OS: Unspecified → Linux
Product: Firefox → Core
Hardware: Unspecified → All

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

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

Flags: needinfo?(gpascutto)
Flags: needinfo?(gpascutto) → needinfo?(aros)
$ cat /etc/ld.so.conf 
include ld.so.conf.d/*.conf

$ cat /etc/ld.so.conf.d/local-x86_64.conf 
/usr/local/lib64
Flags: needinfo?(aros)

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.

Summary: [Linux] Add /usr/local/lib and /usr/local/lib64 to SandBox shared libraries locations → [Linux] /etc/ld.so.conf includes (/etc/ld.so.conf.d/*) management is missing
Assignee: nobody → gpascutto
Priority: -- → P2

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

Summary: [Linux] /etc/ld.so.conf includes (/etc/ld.so.conf.d/*) management is missing → [Linux] /usr/local/(lib|lib64) not picked up from linker config

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)

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.

Confirmed that this is the problem.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: P2 → P1
Summary: [Linux] /usr/local/(lib|lib64) not picked up from linker config → [Linux] relative paths in linker includes not handled correctly

(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.

Flags: needinfo?(jld)

jld is already tagged for the review.

Flags: needinfo?(jld)
Pushed by gpascutto@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/73935c7cf465
Handle relative paths in linker config parsing. r=jld
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70

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 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:
Flags: needinfo?(gpascutto)
Attachment #9079277 - Flags: approval-mozilla-beta?

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

Flags: needinfo?(aros)

(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!

Flags: needinfo?(aros)

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.

Attachment #9079277 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

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

Flags: qe-verify+

Marking bug as verified based on comment 18.

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

Status: RESOLVED → VERIFIED

(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.

Awesome!
Thank you for the help.

You need to log in before you can comment on or make changes to this bug.