1848615 - SetDefaultTimeZoneFromHostTimeZone uses wrong timezone in content process

Status: RESOLVED FIXED

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

Description

[Tom Schuster]

I noticed that new Intl.DateTimeFormat().resolvedOptions().timeZone returns Europe/Oslo on websites such as jsconsole.com, instead of (correctly) Europe/Berlin like in the browser console.

This seems to be caused by icu::TimeZone::detectHostTimeZone returning a different timezone in content processes.

In the parent process detectHostTimeZone() returns Europe/Berlin, but in the content process it's Arctic/Longyearbyen, which is later canonicalized to Europe/Oslo by tryCanonicalizeTimeZoneConsistentWithIANA.

Comment 1

[André Bargull [:anba]]

Does it work correctly when the content sandbox is disabled via MOZ_DISABLE_CONTENT_SANDBOX=1? Bug 1839287 mentions that the content sandbox can cause issues when trying to resolve the host time zone.

Comment 2

[Tom Schuster]

Thanks. I does seem like disabling sandboxing makes it work.

Comment 3

[Tom Schuster]

Attachment: MOZ_SANDBOX_LOGGING

Comment 4

[Tom Schuster]

Attachment: Bug 1848615 - Allow /usr/share/zoneinfo in Linux sandbox ContentPolicy. r?jld

Comment 5

[Tom Schuster]

For reference I am running "Linux Mint 21.2 Cinnamon", and not nix.

Comment 6

[Gian-Carlo Pascutto [:gcp]]

/usr/share and everything below it are already allowed in the sandbox, so just adding that shouldn't change anything. The log shows:

[89518] Sandbox: Failed errno -22 op readlink flags 00 path /etc
[89248] Sandbox: Recording mapping /usr/share/zoneinfo/Europe/Berlin -> /etc/localtime
[89248] Sandbox: SandboxBroker: denied op=readlink rflags=0 perms=0 path=/usr for pid=89518

There's something about the way that the links are laid out here that our code considers insecure, but it's not so obvious what. Unlike the Nix example in the other bug, this is the same setup as there currently is on Ubuntu LTS, so all files are in normal paths (no /nix/store).

Comment 7

[Tom Schuster]

Just speculating, but maybe we need to add "/usr" considering the path=/usr in the error? Edit: Yeah that works. Not sure if there is anything important in there though.

Comment 8

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

Yes, it looks like we could do AddPath("/usr", MAY_READ), and that should work. It would also allow listing the directory, but that should be relatively low-impact.

Slightly outside the scope of this bug, but maybe readlink ought to require only MAY_ACCESS and not MAY_READ, in light of how Linux itself doesn't recognize symlink permissions (i.e., in the absence of our sandboxing, if you can lstat a path then you can also readlink it), so that we don't have to give out readdir access when we really just want realpath to work. However, that change could have a lot of side effects, so I'd prefer not to block this bug on that.

For reference, here's an example of what realpath is doing:

$ strace -fereadlink,readlinkat realpath /etc/localtime
readlink("/etc", 0x7ffe89c6da20, 1023)  = -1 EINVAL (Invalid argument)
readlink("/etc/localtime", "/usr/share/zoneinfo/US/Pacific", 1023) = 30
readlink("/usr", 0x7ffe89c6da20, 1023)  = -1 EINVAL (Invalid argument)
readlink("/usr/share", 0x7ffe89c6da20, 1023) = -1 EINVAL (Invalid argument)
readlink("/usr/share/zoneinfo", 0x7ffe89c6da20, 1023) = -1 EINVAL (Invalid argument)
readlink("/usr/share/zoneinfo/US", 0x7ffe89c6da20, 1023) = -1 EINVAL (Invalid argument)
readlink("/usr/share/zoneinfo/US/Pacific", "../America/Los_Angeles", 1023) = 22
readlink("/usr/share/zoneinfo/America", 0x7ffe89c6da20, 1023) = -1 EINVAL (Invalid argument)
readlink("/usr/share/zoneinfo/America/Los_Angeles", 0x7ffe89c6da20, 1023) = -1 EINVAL (Invalid argument)
/usr/share/zoneinfo/America/Los_Angeles
+++ exited with 0 +++

On NixOS (see bug 1839287), it looks like it's /etc/localtime → /etc/zoneinfo/<timezone>, then /etc/zoneinfo → /etc/static/zoneinfo, /etc/static → /nix/store/<hash>-etc/etc, then /nix/store/<hash>-etc/etc/zoneinfo → /nix/store/<hash>-tzdata-<version>/share/zoneinfo. So, we'd also want to add /nix similary to /usr. (We already allow read access to /etc and anything under it.)

Comment 9

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

We have the affected ICU version; marking bug 1824744 as the regressor so uplifts can be tracked.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1824744

Comment 11

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

Attachment: Bug 1848615 - Adjust Linux content sandbox policy so that ICU can get the canonical time zone.

ICU version 73 (bug 1824744) has a change to call realpath
rather than just readlink on /etc/localtime, meaning that it needs to
be able to readlink every directory involved in path resolution. In
particular, for a symlink into /usr/share/zoneinfo, this includes
/usr, which is blocked by the content sandbox policy.

Currently, the file broker requires MAY_READ permission to allow
readlink, so we grant that on /usr and /nix (there will be a
similar issue with symlinks to /nix/store/...). Note that this
applies only to those directories themselves, not files within them.

This also means that the process can open those directories for reading
(i.e., readdir), but that should be relatively low-impact compared to
the information that's already exposed.

Comment 13

[Gian-Carlo Pascutto [:gcp]]

However, that change could have a lot of side effects, so I'd prefer not to block this bug on that.

Fair enough. I had similar observations about MAY_ACCESS vs MAY_READ in the patch review, though.

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1824744

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ab673eef69bf
Adjust Linux content sandbox policy so that ICU can get the canonical time zone. r=gcp

Comment 16

[Cosmin Sabou [:CosminS]]

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

Comment 17

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:jld, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox118 to wontfix.

For more information, please visit BugBot documentation.

Comment 18

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

Comment on attachment 9349651 [details]
Bug 1848615 - Adjust Linux content sandbox policy so that ICU can get the canonical time zone.

Beta/Release Uplift Approval Request

• User impact if declined: Incorrect information from JS internationalization APIs (depending on the user's locale / time zone settings) on Linux; see the initial bug report for details.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce: I don't need manual testing (the automated test I added should be enough), but if it matters, I was able to reproduce the bug by changing my system time zone to Europe/Berlin and evaluating the JS expression in the bug report.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This makes the sandbox policy more permissive, so it's unlikely to cause any functional regressions; and the addition to the sandbox policy is fairly small, so it's unlikely to unexpectedly regress the effectiveness of sandboxing.
• String changes made/needed: none
• Is Android affected?: No

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is a regression from earlier versions, it's a change in Web API behavior exposed to content, and the risk of uplift should be low.
• User impact if declined: (See Beta uplift request.)
• Fix Landed on Version: 119
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): (See beta uplift request.)

Comment 20

[Donal Meehan [:dmeehan]]

Comment on attachment 9349651 [details]
Bug 1848615 - Adjust Linux content sandbox policy so that ICU can get the canonical time zone.

Approved for 118.0b3

Comment 21

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/8e86702e04b6

Comment 22

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/d7948b142f43

Comment 23

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-esr115/rev/d7948b142f43

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9349651 [details]
Bug 1848615 - Adjust Linux content sandbox policy so that ICU can get the canonical time zone.

Approved for 117.0.1

Comment 25

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/5da351867d7e

Comment 26

[Ryan VanderMeulen [:RyanVM]]

Added to the 117.0.1 release notes:

Fixed an issue causing incorrect time zones to be detected on some sites