Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

treewide: Fix unsafe concatenation of $LD_LIBRARY_PATH #76804

Merged
merged 1 commit into from
Jan 15, 2020
Merged

treewide: Fix unsafe concatenation of $LD_LIBRARY_PATH #76804

merged 1 commit into from
Jan 15, 2020
+55 −55

Conversation

andersk
Copy link
Contributor

@andersk andersk commented Jan 2, 2020

Motivation for this change

Naive concatenation of $LD_LIBRARY_PATH can result in an empty colon-delimited segment; this tells glibc to load libraries from the current directory, which is definitely wrong, and may be a security vulnerability if the current directory is untrusted. (See #67234, for example.) Fix this throughout the tree.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

Sorry, something went wrong.

@andersk andersk mentioned this pull request Jan 2, 2020
Open
andersk
andersk commented Jan 2, 2020
View reviewed changes
pkgs/development/python-modules/pytorch/default.nix
"LD_LIBRARY_PATH=${cudaStub}\${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} ";
"LD_LIBRARY_PATH=${cudaStub}\${LD_LIBRARY_PATH:+:}$LD_LIBRARY_PATH ";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(This one is a no-op just to match the more common style, so that real problems can be found more reliably by grepping for :$LD_LIBRARY_PATH and $LD_LIBRARY_PATH:.)

@veprbl
Copy link
Member

veprbl commented Jan 2, 2020

This will cause mass-rebuild so it should target staging.

@ofborg ofborg bot added 6.topic: haskell 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: python 6.topic: qt/kde 6.topic: steam Steam game store/launcher (store.steampowered.com) 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ labels Jan 2, 2020
@andersk andersk changed the base branch from master to staging January 2, 2020 01:09
@cdepillabout
Copy link
Member

pkgs/development/haskell-modules/hackage-packages.nix is generated automatically and should not be changed by hand.

It is generated by https://github.com/NixOS/cabal2nix. Please file an issue on that repo if things aren't being generated correctly. (It looks like maybe something isn't being escaped correctly?)

@andersk andersk mentioned this pull request Jan 2, 2020
Merged
@andersk
treewide: Fix unsafe concatenation of $LD_LIBRARY_PATH
b0c846b 
Naive concatenation of $LD_LIBRARY_PATH can result in an empty
colon-delimited segment; this tells glibc to load libraries from the
current directory, which is definitely wrong, and may be a security
vulnerability if the current directory is untrusted.  (See NixOS#67234, for
example.)  Fix this throughout the tree.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
@andersk
Copy link
Contributor Author

andersk commented Jan 2, 2020

@cdepillabout Thanks, I’ve removed that change and opened NixOS/cabal2nix#438.

@ofborg ofborg bot removed the 6.topic: haskell label Jan 2, 2020
jonringer
jonringer approved these changes Jan 3, 2020
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

diff LGTM

@FRidh FRidh merged commit 3cd8ce3 into NixOS:staging Jan 15, 2020
@samdoshi samdoshi mentioned this pull request Feb 1, 2020
Closed
10 tasks
samdoshi added a commit to samdoshi/nixpkgs that referenced this pull request Feb 1, 2020
@samdoshi
opencc: fix broken build
c5ccad8 
The LD_LIBRARY_PATH update from 3cd8ce3 causes opencc to stop building.

See also: NixOS#76804
andersk added a commit to andersk/nixpkgs that referenced this pull request May 31, 2020
@andersk
libversion: Fix unsafe concatenation of $LD_LIBRARY_PATH
665dfc2 
Naive concatenation of $LD_LIBRARY_PATH can result in an empty
colon-delimited segment; this tells glibc to load libraries from the
current directory, which is definitely wrong, and may be a security
vulnerability if the current directory is untrusted.  This particular
case probably has no security relevance, but we should avoid this
unsafe pattern anyway in case it gets copied.  See NixOS#76804.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
@andersk andersk mentioned this pull request May 31, 2020
Merged
10 tasks
@andersk andersk mentioned this pull request Nov 4, 2021
Closed
@andersk andersk deleted the LD_LIBRARY_PATH branch November 4, 2021 22:44
andersk added a commit to andersk/nixpkgs that referenced this pull request Nov 4, 2021
@andersk
treewide: Fix unsafe concatenation of $LD_LIBRARY_PATH, round 2
e12f4db 
Naive concatenation of $LD_LIBRARY_PATH can result in an empty
colon-delimited segment; this tells glibc to load libraries from the
current directory, which is definitely wrong, and may be a security
vulnerability if the current directory is untrusted.  (See NixOS#67234, for
example.)  Fix this throughout the tree.

Followup to NixOS#76804.  Fixes NixOS#144646.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
@andersk andersk mentioned this pull request Nov 4, 2021
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonringer jonringer

@basvandijk basvandijk

@cdepillabout cdepillabout

@FRidh FRidh

@infinisil infinisil

@matthewbauer matthewbauer

@ttuegel ttuegel

Assignees
No one assigned
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: python 6.topic: qt/kde 6.topic: steam Steam game store/launcher (store.steampowered.com) 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
Staging
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@andersk @veprbl @cdepillabout @jonringer @FRidh