
How Torch broke ls and made me vulnerable - joshumax
https://joshumax.github.io/general/2017/06/08/how-torch-broke-ls.html
======
binarycrusader
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...](https://enchildfone.wordpress.com/2010/03/23/a-description-
of-rpath-origin-ld_library_path-and-portable-linux-binaries/)
[http://man7.org/linux/man-pages/man8/ld.so.8.html](http://man7.org/linux/man-
pages/man8/ld.so.8.html)
[http://longwei.github.io/rpath_origin/](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
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
RPATH is very nice, but it's a _huge_ pain to set in Makefiles because you
have to reliably escape the "$ORIGIN"

~~~
Spivak
(Don't do this) You could patch the binary after compilation with elfpatch.

Also,

test:

    
    
      echo "\$$ORIGIN"
    

outputs $ORIGIN. $$ translates to a literal dollar sign. \ escapes the dollar
sign in the shell.

~~~
binarycrusader
You can use elfedit instead, but really, it's not that much of a pain to just
do it right in the first place.

------
anderskaseorg
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](https://github.com/torch/distro/pull/228).

~~~
djsumdog
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.

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

------
AceJohnny2
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](http://man7.org/linux/man-
pages/man8/ld.so.8.html)

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

~~~
zwp
> 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 exec _p_ () 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.

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

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

~~~
bartread
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.

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

------
thewisenerd
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/...](https://github.com/omnirom/android_bootable_recovery/commit/9c3b7e990e162319cb379545b458838478a19eb0)

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

~~~
eridius
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.

~~~
lloeki
> 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](http://underhanded-c.org)

~~~
eridius
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.

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

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

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

But you're right it should not occur.

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

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

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

Thanks for sharing!

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

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

