Torch probably doesn't even need to set LD_LIBRARY_PATH. If LD_LIBRARY_PATH is only being set so that binaries distributed by torch work, then I'd strongly suggest they use RUNPATH instead with $ORIGIN.
This nice and clean solution is too little known I think. Far better than shipping shell scripts which are very hard to get right and most application developers are not shell scripting experts.
I've wrestled with this in the past, and I eventually got things working to my satisfaction by reading the man page, but these blog posts bring a lot of clarity. Thanks for linking them!
Good fix and good for sending them a merge request.
I still find it kinda baffling glibc would have this behavior for a trailing colon (:). Like, I know it's probably legacy/comparability, but it feels like a security nightmare. ./ should be explicit, not implicit.
> Sounds like a bug in GNU's ld.so more than anything.
It's neither unique to glibc (AIX, Solaris) nor to LD_LIBRARY_PATH (PATH), nor trailing colons (leading colons, adjacent colons).
This de facto standard becomes a little more obvious when one considers a likely implementation (iterating over "strchr(arg, ':')" or whatever). Any of these sequences then will give up an empty string:
PATH=:/foo
PATH=/foo:
PATH=/foo::/bar
And an empty string is equivalent to dot for chdir(2).
zwp:/tmp$ cd ''
zwp:/tmp$ pwd
/tmp
zwp:/tmp$
(This is not the same as plain "cd" (ie with no args), which is a special case that takes you $HOME, of course).
I agree it's surprising and potentially dangerous.
FWIW, the execp() functions hide a similar wtf. From the Linux man page:
The file is sought in the colon-separated list of
directory pathnames specified in the PATH envi‐
ronment variable. If this variable isn't defined,
the path list defaults to the current directory
followed by the list of directories returned by
confstr(_CS_PATH).
Security conscious programs that clear the environment and then call eg execlp() end up searching dot before the system path. Yay.
The dynamic array tag DT_RUNPATH gives a string that holds a list of directories, separated by colons (:). For example, the string /home/dir/lib:/home/dir2/lib: tells the dynamic linker to search first the directory /home/dir/lib, then /home/dir2/lib, and then the current directory to find dependencies.
The following values would be equivalent to the previous example:
Even if it was clearly documented, it's still a terrible idea. The OS should try to make it difficult, ideally impossible, to accidentally introduce vulnerabilities.
Same. I was irritated enough by the badly formed headline to read the story to try and figure out what was going on.
What's wrong with something like, "Torch machine learning introduces vulnerability in loading of shared libraries." Whilst it doesn't tell the whole story it does at least give a flavour instead of just sowing confusion.
On most modern Mac OS installations, this is a non-issue. System Integrity Protection doesn't honor any changes to LD_LIBRARY_PATH, presumably for exactly this sort of reason. (Of course, one might have turned off SIP, in which case this is no longer true, but it's nice to know it's the default).
Very sensible. I wish OS X featured a even more "rootless" mode where "root" only gives you sandboxed write access, and read access to files it creates.
on a similar note, this (having '.' or $(pwd) on LD_LIBRARY_PATH) also broke the `ls` command (and a bunch of other stuff) in the TeamWin Recovery Project (TWRP) recovery on mobile devices when you were in `/system/lib` on a 64-bit machine.
How is it a backdoor? System services don't typically source the user's bash profile before running, and even if they did, they don't run from attacker-controlled directories anyway. At best you could compromise someone by tricking them into cd'ing into a folder you provided, but that's not something that would generally be called a "backdoor". And if you can get them to run your install script, you've already "compromised" them anyway and LD_LIBRARY_PATH is completely unnecessary.
> And if you can get them to run your install script
If your script is obviously malicious then you're reducing your chances. Such a change could seem innocuous[0], then, cloning a repo containing a so file in the middle of a long list and cd'ing would trigger payload execution. Distributing the maliciousness by chaining innocuously looking actions is both effective at bypassing human logical analysis and plausibly deniable (up to a point).
If you clone a repo and cd into it, that's because you're going to actually do something in there. An install script that clones a repo, cd's into it, and then does nothing is extremely suspicious. But a script that clones a repo, cd's into it, and runs `make install` isn't particularly suspicious, so once again, there's no need for LD_LIBRARY_PATH.
with several browsers happy to automatically download whatever is provided to them, making a user download a libblahblah.so is relatively easy, and unless the user was aware of the ramifications of just even having such a file in a folder, might ls, cat, grep, etc in that folder like they do any other day
However, most of the latest versions of the mainstream browsers now intentionally avoid writing out such files to the downloads folder unless the end user specifically OKs it (due to situations like this!)
It's a plausibly-deniable backdoor on top of LD_LIBRARY_PATH, just to clarify. As in - oops, I added the colon at the end, or if the LD_LIBRARY_PATH was programmatically generated, a sloppy loop can clearly do that...and now you're stuck with unexpected behavior which, while not directly unsafe, can be coopted in a nefarious manner. The $PWD behavior is niche, unexpected and likely to be missed in an audit. If you have software that relies of LD_LIBRARY_PATH and you distribute the OS, I'd change ld.so to avoid the $PWD behavior.
It's a very unreliable attack vector, but what makes it completely pointless is in order to even set it up, you already need permission to edit the user's bash profile, and if you can do that, then you don't need LD_LIBRARY_PATH.
Why is torchs install so weird? Why can't it use standard package management, and why does it need to install to my home directory? I'm not surprised to see it causes security issues.
Maybe, but this particular issue has much wider scope, and is only incidentally a Torch issue. A disclosure by the Torch devs might have gone unnoticed by those who are not Torch users - I only read it because the HN title mentioned ls, and I thought "that looks odd...".
> Ask yourself if this could have happened with Windows 10's PATH editor.
Yes …? Well, I dunno; I don't know how Windows 10's PATH editor works. Nonetheless, the issue seems to be with magic interpretation of special configuration options, not with how those configuration options are entered. (Note also that the configuration was done programmatically, not by the user, so that there would have to be some kind of parse–deparse step anyway.)
There are examples in various places:
https://enchildfone.wordpress.com/2010/03/23/a-description-o... http://man7.org/linux/man-pages/man8/ld.so.8.html http://longwei.github.io/rpath_origin/
LD_LIBRARY_PATH is really only for a developer's local use; it should never be used for installed software.
Disclaimer: may not apply in some scenarios, I haven't used Torch, so this is merely a general observation.