Skip to content

libstore: guard against empty archive in unpack paths#15242

Merged
xokdvium merged 1 commit intoNixOS:masterfrom
obsidiansystems:fix-prefetch-segfault
Feb 22, 2026
Merged

libstore: guard against empty archive in unpack paths#15242
xokdvium merged 1 commit intoNixOS:masterfrom
obsidiansystems:fix-prefetch-segfault

Conversation

@amaanq
Copy link
Member

@amaanq amaanq commented Feb 15, 2026

Motivation

DirectoryIterator is dereferenced without an end check, which segfaults when unpacking an empty or zero-sized archive. This commit adds an emptiness check before the dereference in both prefetchFile and builtinUnpackChannel, throwing a descriptive error instead.

Context


Add 👍 to pull requests you find important.

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

`DirectoryIterator` is dereferenced without an end check, which segfaults
when unpacking an empty or zero-sized archive. This commit adds an
emptiness check before the dereference in both `prefetchFile` and
`builtinUnpackChannel`, throwing a descriptive error instead.
Fixes NixOS#15116.
@github-actions github-actions bot added new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority labels Feb 15, 2026
@Ericson2314
Copy link
Member

@xokdvium should we have some sort of empty or end method on DirectoryIterator to be compliant?

@xokdvium
Copy link
Contributor

should we have some sort of empty or end method on DirectoryIterator to be compliant?

The current situation is fine. It's not a range, but an iterator, so it's all good. Think std::istream_iterator.

Comment on lines +134 to +135
if (entries == DirectoryIterator{})
throw Error("archive '%s' is empty", url.to_string());
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that old versions (like 2.18) created an empty file. Not sure what's the best way to address this is.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure at all, though I guess it was a bug/unintended side effect that there were empty archives created before.

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably, can we discuss the intended behaviour in the linked issue?

Copy link
Contributor

@xokdvium xokdvium left a comment

Choose a reason for hiding this comment

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

The builtin builder change is good regardless. I think it's fine to stick with error out in all code paths too.

@xokdvium xokdvium added this pull request to the merge queue Feb 22, 2026
Merged via the queue into NixOS:master with commit b1ad42e Feb 22, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new-cli Relating to the "nix" command 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.

nix-prefetch-url --unpack segfaults with zero sized file as input

3 participants