dom96 / choosenim

Tool for easily installing and managing multiple versions of the Nim programming language.
BSD 3-Clause "New" or "Revised" License
679 stars 67 forks source link

Fix a PATH hijacking issue #308

Closed the-emmons closed 1 year ago

the-emmons commented 1 year ago

Currently, the Unix installer encourages an unsafe action that can result in hijacking of the PATH variable. By setting the writable nimble directory as the first search location over standard locations like /bin and /usr/bin, an attacker can hijack PATH searches. Instead of prepending to the path, the new entry should be appended.

dom96 commented 1 year ago

What does the PATH hijacking involve? Got any links that describe it well?

The problem with this change is that if the user has Nim installed via their package manager, the choosenim-installed nim will not be used.

the-emmons commented 1 year ago

@dom96

What does the PATH hijacking involve? Got any links that describe it well?

In a nutshell, it's a privilege escalation or lateral movement that involves creating malicious binaries in the writable early PATH directories. If a cronjob or shell script regularly runs cat /etc/passwd, an attacker could create a malicious shell script named 'cat' and drop it into the writable Nim directory. As a result, the cat binary would be found before the real /bin/cat binary, resulting in that running instead. Further reading with abuse examples: https://www.linuxfordevices.com/tutorials/linux/path-environment-variable

The problem with this change is that if the user has Nim installed via their package manager, the choosenim-installed nim will not be used.

What about noting this in the terminal message to ensure users know that they should use one or the other, not both? You might also script the /usr/bin/which nim command to check if it's already resolvable on the path, then prompt to uninstall if it's found. Right now, the default is insecure. If the above were implemented, the insecure configuration would be opt-in instead.

Happy to help out in any way I can with this.

dom96 commented 1 year ago

From what I can see, rustup seems to do the same thing:

$ cat $HOME/.cargo/env
#!/bin/sh
# rustup shell setup
# affix colons on either side of $PATH to simplify matching
case ":${PATH}:" in
    *:"$HOME/.cargo/bin":*)
        ;;
    *)
        # Prepending path in case a system-installed rustc needs to be overridden
        export PATH="$HOME/.cargo/bin:$PATH"
        ;;
esac

And it does it automatically too. Choosenim at least let's the user decide how they wish to configure the PATH.

Since other tools seem to not worry about this class of problem I'd rather not complicate choosenim's logic either.

the-emmons commented 1 year ago

That's reasonable, thanks for taking the time to consider!