Skip to content

libutil: Make RestoreSink use *at system calls on UNIX#14597

Merged
xokdvium merged 4 commits intomasterfrom
restore-sink-openat
Nov 20, 2025
Merged

libutil: Make RestoreSink use *at system calls on UNIX#14597
xokdvium merged 4 commits intomasterfrom
restore-sink-openat

Conversation

@xokdvium
Copy link
Contributor

@xokdvium xokdvium commented Nov 19, 2025

Motivation

This is necessary to ban symlink following. It can be considered
a defense in depth against issues similar to CVE-2024-45593.

Context


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

…UL bytes better

Same utility as in lix's change I3caf476e59dcb7899ac5a3d83dfa3fb7ceaaabf0.

Co-authored-by: eldritch horrors <pennae@lix.systems>
@xokdvium xokdvium requested a review from edef1c November 19, 2025 23:16
@xokdvium xokdvium requested a review from edolstra as a code owner November 19, 2025 23:16
@xokdvium xokdvium force-pushed the restore-sink-openat branch from 9a56b89 to cb69b13 Compare November 19, 2025 23:40
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Nov 19, 2025
Copy link
Member

@edef1c edef1c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As stated in Nix team meeting, definite improvement, and seems good to merge.
Longer-term, I'd like us to get rid of the open()s and have the caller pass in the parent dir fd, and preferably only do single-component O_NOFOLLOW openats, since O_NOFOLLOW only applies to the final component.

@Ericson2314 Ericson2314 added this pull request to the merge queue Nov 20, 2025
@xokdvium xokdvium removed this pull request from the merge queue due to a manual request Nov 20, 2025
This is necessary to ban symlink following. It can be considered
a defense in depth against issues similar to CVE-2024-45593. By
slightly changing the API in a follow-up commit we will be able
to mitigate the symlink following issue for good.
Now the intermediate symlink following issue should be completely plugged.
@xokdvium xokdvium force-pushed the restore-sink-openat branch from cb69b13 to 40b2515 Compare November 20, 2025 01:01
@xokdvium
Copy link
Contributor Author

Just noticed that I've missed some CLOEXEC. Added them, though I doubt it would matter in practice

@xokdvium xokdvium enabled auto-merge November 20, 2025 01:07
@xokdvium xokdvium added this pull request to the merge queue Nov 20, 2025
Merged via the queue into master with commit 70b9fbd Nov 20, 2025
20 checks passed
@xokdvium xokdvium deleted the restore-sink-openat branch November 20, 2025 02:34
@edolstra edolstra mentioned this pull request Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants