How Torch broke ls and made me vulnerable

https://joshumax.github.io/general/2017/06/08/how-torch-broke-ls.html

by joshumax • 9 years ago

183 36 9 years ago

joshumax.github.io

fslkjhjdfhgj4j• 9 years ago

wow! thats a gotcha, trailing : appends $(PWD) to the LD_LIBRARY_PATH

Thanks for sharing!

andreiw2• 9 years ago

you know, that's a pretty good plausibly-deniable backdoor, if you think about it...

sounds like a pretty good thing to disable in ld.so...

eridius3• 9 years ago

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.

RubyPinch• 9 years ago

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

lloeki• 9 years ago

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

[0] http://underhanded-c.org

andreiw2• 9 years ago

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.

thewisenerd• 9 years ago

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.

https://github.com/omnirom/android_bootable_recovery/commit/...

anderskaseorg• 9 years ago

How to safely prepend a directory to a PATH-like variable (in any POSIX-compliant shell):

export LD_LIBRARY_PATH=/opt/whatever/lib${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}

Pull request sent to https://github.com/torch/distro/pull/228.

djsumdog• 9 years ago

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.

emmelaich2• 9 years ago

Also for leading colons. (but you probably knew that)

foota• 9 years ago

Seems like this may almost have been better done through a disclosure channel with torch?

parmesan• 9 years ago

Indeed, first thing alert the devs and let them sort it out..

mannykannot• 9 years ago

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...".

binarycrusader2• 9 years ago

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.

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.

catdog• 9 years ago

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.

pjc50• 9 years ago

RPATH is very nice, but it's a huge pain to set in Makefiles because you have to reliably escape the "$ORIGIN"

sevensor• 9 years ago

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!

matheweis• 9 years ago

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

HappyTypist• 9 years ago

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.

lloydde• 9 years ago

Buried in there is "Torch machine learning library", which then allowed me to figure out what software was being blamed. http://torch.ch/

ChristianGeek• 9 years ago

At least you didn't read "ls " as "is" with a capital "i"...I only read it it to un-confuse myself!

bartread• 9 years ago

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.

AceJohnny2• 9 years ago

More concerning to me is that ld.so will interpret a trailing `:` in LD_LIBRARY_PATH to mean to include PWD.

Where is this documented? It's not indicated in ld.so's manpage:

http://man7.org/linux/man-pages/man8/ld.so.8.html

Sounds like a bug in GNU's ld.so more than anything.

ratmice• 9 years ago

It is documented in the elf specification (however I agree it should be documented or preferably excised from ld.so)

http://www.sco.com/developers/gabi/latest/ch5.dynamic.html#s...

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:

LD_LIBRARY_PATH=/home/dir/usr/lib:/home/dir2/usr/lib:

pawadu• 9 years ago

Note that this happens anytime you mistype a variable, e.g.

    export FOOBAR=/usr/local/lib
    export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$FOOBAL

So yeah, fix the source of the problem in ld.so, don't blame it on Torch.

DarkKomunalec• 9 years ago

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.

zwp• 9 years ago

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.

IshKebab• 9 years ago

Ha, shitty text-based configuration systems strike again. Ask yourself if this could have happened with Windows 10's PATH editor.

JadeNB• 9 years ago

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

tebruno99• 9 years ago

So I'm less concerned about Torch and more concerned about GNU ld adding things unexpected. This sort of "magic" shouldn't occur.

emmelaich2• 9 years ago

It's not specific to GNU ld, it is standard UNIX behaviour. Always has been.

But you're right it should not occur.

Houshalter• 9 years ago

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.

How Torch broke ls and made me vulnerable

Related Stories