1796023 - (CVE-2023-6857) nsLocalFile::GetNativeTarget can silently truncate the link target

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

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

The basic problem is the same as in bug 1791029 — we don't check if the buffer passed to readlink was actually big enough. If there's a race with changing the symlink, or if lstat doesn't give the actual length (e.g., Linux procfs): bug 1791029 took care of the case where the actual value is shorter than expected, but if it's longer than expected, we'll silently truncate it.

The approach that GNU ls seems to take is: allocate the buffer 1 byte longer than expected, so we can distinguish the expected result from the string being longer, and then in case of overflow, double the buffer length and retry until it works. GNU readlink, in contrast, doesn't stat the symlink and just starts with 64 and doubles the buffer size until it fits.

Comment 1

[Gian-Carlo Pascutto [:gcp]]

Barret, are you still working on this?

Comment 2

[Beth Rennie [:beth] (she/her)]

Yes, this fell off my plate, but I'll put a patch up soon.

Comment 3

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1796023 - Compare return value of readlink in nsLocalFileUnix::GetNativeTarget r?#xpcom-reviewers

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bc1ea88587af
Compare return value of readlink in nsLocalFileUnix::GetNativeTarget r=xpcom-reviewers,nika

Comment 5

[Ryan VanderMeulen [:RyanVM]]

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

Comment 6

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

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

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

For more information, please visit BugBot documentation.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Please request ESR115 approval on this when you get a chance. It grafts cleanly.

Comment 8

[Beth Rennie [:beth] (she/her)]

Comment on attachment 9358412 [details]
Bug 1796023 - Compare return value of readlink in nsLocalFileUnix::GetNativeTarget r?#xpcom-reviewers

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: The patch is small and grafts cleanly.
• User impact if declined: Low to no impact.
• Fix Landed on Version: 121
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is not risky becuase this patch has ridden all the way to beta with no issues.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9358412 [details]
Bug 1796023 - Compare return value of readlink in nsLocalFileUnix::GetNativeTarget r?#xpcom-reviewers

Approved for 115.6esr.

Comment 10

[Pulsebot]

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

Comment 11

[Jason Kratzer [:jkratzer]]

Attachment: advisory.txt

Comment 12

[Jason Kratzer [:jkratzer]]

Attachment: advisory.txt

Comment 13

[Jason Kratzer [:jkratzer]]

Attachment: advisory.txt

Comment 14

[Jason Kratzer [:jkratzer]]

Attachment: advisory.txt

Comment 15

[Daniel Veditz [:dveditz]]

Bulk-unhiding security bugs fixed in Firefox 119-121 (Fall 2023). Use "moo-doctrine-subsidy" to filter