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.

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

treewide: Fix unsafe concatenation of $LD_LIBRARY_PATH #76804

Merged
merged 1 commit into from Jan 15, 2020

Conversation

@andersk
Copy link
Contributor

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 @

"LD_LIBRARY_PATH=${cudaStub}\${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} ";
"LD_LIBRARY_PATH=${cudaStub}\${LD_LIBRARY_PATH:+:}$LD_LIBRARY_PATH ";
Comment on lines -108 to +108

This comment has been minimized.

Copy link
@andersk

andersk Jan 2, 2020

Author Contributor

(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

This comment has been minimized.

Copy link
Member

veprbl commented Jan 2, 2020

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

@cdepillabout

This comment has been minimized.

Copy link
Member

cdepillabout commented Jan 2, 2020

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?)

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.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
@andersk andersk force-pushed the andersk:LD_LIBRARY_PATH branch from d0b29cf to b0c846b Jan 2, 2020
@andersk

This comment has been minimized.

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
Staging automation moved this from Needs review to Ready Jan 3, 2020
Copy link
Contributor

jonringer left a comment

diff LGTM

@FRidh FRidh merged commit 3cd8ce3 into NixOS:staging Jan 15, 2020
13 checks passed
13 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
Staging automation moved this from Ready to Done Jan 15, 2020
samdoshi added a commit to samdoshi/nixpkgs that referenced this pull request Feb 1, 2020
The LD_LIBRARY_PATH update from 3cd8ce3 causes opencc to stop building.

See also: NixOS#76804
@samdoshi samdoshi mentioned this pull request Feb 1, 2020
4 of 10 tasks complete
samdoshi added a commit to samdoshi/nixpkgs that referenced this pull request Feb 1, 2020
The LD_LIBRARY_PATH update from 3cd8ce3 causes opencc to stop building.

See also: NixOS#76804
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.