makeBinaryWrapper: fix read past NUL - #541663
Merged
Merged
Conversation
nixpkgs-ci
Bot
added
10.rebuild-linux: 501+
LeSuisse
added
1.severity: security
nixpkgs-ci
Bot
removed
the
9.needs: reviewer
ncfavier
requested changes
Jul 23, 2026
accelbread
force-pushed
the
binwrapper-fix
branch
from
August 4, 2026 23:55
05918b7 to
c0b72db
Compare
nixpkgs-ci
Bot
added
the
2.status: merge conflict
When setting a prefix for a path-like environment variable, the deduplication code in set_env_prefix reads past the NUL byte at the end of the env val and into the next entry. This corrupts the resultant env value with data from the next env var, or other data sitting after it.
accelbread
force-pushed
the
binwrapper-fix
branch
from
August 5, 2026 01:02
c0b72db to
d75cae8
Compare
nixpkgs-ci
Bot
removed
the
2.status: merge conflict
ncfavier
approved these changes
Aug 5, 2026
nixpkgs-ci
Bot
added
the
12.approvals: 1
Contributor
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin staging-26.05
git worktree add -d .worktree/backport-541663-to-staging-26.05 origin/staging-26.05
cd .worktree/backport-541663-to-staging-26.05
git switch --create backport-541663-to-staging-26.05
git cherry-pick -x d75cae80fa4ab7b5f6f39ddcbbbfbaa1a9183d81 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes a bug in makeBinaryWrapper which corrupts path-like environment variables when using the prefix option and the path had an existing duplicate element in the last element. In this case the nul byte is skipped and the value is overread (likely into the next element).
This bug causes issues as env vars intended values get concatenated with neighboring values.
On my system this causes Gstreamer directories to be appended onto GIO modules path. This causes every GIO application to dlload all gstreamer modules, creating a flood of error logs, and also running initialization code in each of those modules.
This was also interesting to debug as the env variables passed to the program look completely correct since they are corrupted by makeBinaryWrapper; observing what pam outputs or what gets passed to the program look correct. This allows manipulating path-like env vars such that they appear different to the application executing the program and the actual program under the wrapper.
I have updated the makeBinaryWrapper tests as needed as part of the change commit. I also added a new test for this case.
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.