Message ID | mvma9cfobqi.fsf@hawking.suse.de |
---|---|
State | New |
Headers | show |
On Mon, Mar 24, 2014 at 04:30:29PM +0100, Andreas Schwab wrote: > Executing a random file is never a good idea. Treat all arguments as if > they are invoked with __libc_enable_secure, and run them through the known > good dynamic linker. > > * elf/ldd.bash.in: Always run through the dynamic linker, even if > the file has its own interpreter. Remove unneeded executable > check. I've been rebasing this fix since 2002. Please commit. > --- > elf/ldd.bash.in | 16 +--------------- > 1 file changed, 1 insertion(+), 15 deletions(-) > > diff --git a/elf/ldd.bash.in b/elf/ldd.bash.in > index 4ff140d..3986bcf 100644 > --- a/elf/ldd.bash.in > +++ b/elf/ldd.bash.in > @@ -150,8 +150,6 @@ for file do > echo "ldd: ${file}:" $"not regular file" >&2 > result=1 > elif test -r "$file"; then > - test -x "$file" || echo 'ldd:' $"\ > -warning: you do not have execution permission for" "\`$file'" >&2 > RTLD= > ret=1 > for rtld in ${RTLDLIST}; do > @@ -164,18 +162,6 @@ warning: you do not have execution permission for" "\`$file'" >&2 > fi > done > case $ret in > - 0) > - # If the program exits with exit code 5, it means the process has been > - # invoked with __libc_enable_secure. Fall back to running it through > - # the dynamic linker. > - try_trace "$file" > - rc=$? > - if [ $rc = 5 ]; then > - try_trace "$RTLD" "$file" > - rc=$? > - fi > - [ $rc = 0 ] || result=1 > - ;; > 1) > # This can be a non-ELF binary or no binary at all. > nonelf "$file" || { > @@ -183,7 +169,7 @@ warning: you do not have execution permission for" "\`$file'" >&2 > result=1 > } > ;; > - 2) > + [02]) > try_trace "$RTLD" "$file" || result=1 > ;; > *)
I always thought it wrong that it did that too. But I vaguely recall being told there was some reason for it. (Maybe even I thought myself there was an adequate reason. I can't recall any details now.) So we should understand what the past reasoning was and be sure it no longer applies today before we make such a change. The only thing that comes to mind is cases where PT_INTERP points to a different dynamic linker, such as a from build with a special --prefix= setup or something stranger. In those cases, what the vanilla rtld will think about search paths and so forth won't match what the actual PT_INTERP dynamic linker will do. But I'm not at all sure that was the case (or was the only case) that motivated the current behavior.
On Mon, Mar 24, 2014 at 03:10:23PM -0700, Roland McGrath wrote: > I always thought it wrong that it did that too. But I vaguely recall being > told there was some reason for it. (Maybe even I thought myself there was > an adequate reason. I can't recall any details now.) So we should > understand what the past reasoning was and be sure it no longer applies > today before we make such a change. > > The only thing that comes to mind is cases where PT_INTERP points to a > different dynamic linker, such as a from build with a special --prefix= > setup or something stranger. In those cases, what the vanilla rtld will > think about search paths and so forth won't match what the actual PT_INTERP > dynamic linker will do. > > But I'm not at all sure that was the case (or was the only case) that > motivated the current behavior. If there's really a need to support this kind of usage, I think by default ldd should refuse to run when PT_INTERP doesn't match its own idea of the dynamic linker, and should require a --force-run option or something. In the default setup, it's completely non-obvious to most admins that ldd _runs_ the program, and the "hey, root! this program is spewing missing symbol errors!" social-engineering exploit is a real risk. Rich
diff --git a/elf/ldd.bash.in b/elf/ldd.bash.in index 4ff140d..3986bcf 100644 --- a/elf/ldd.bash.in +++ b/elf/ldd.bash.in @@ -150,8 +150,6 @@ for file do echo "ldd: ${file}:" $"not regular file" >&2 result=1 elif test -r "$file"; then - test -x "$file" || echo 'ldd:' $"\ -warning: you do not have execution permission for" "\`$file'" >&2 RTLD= ret=1 for rtld in ${RTLDLIST}; do @@ -164,18 +162,6 @@ warning: you do not have execution permission for" "\`$file'" >&2 fi done case $ret in - 0) - # If the program exits with exit code 5, it means the process has been - # invoked with __libc_enable_secure. Fall back to running it through - # the dynamic linker. - try_trace "$file" - rc=$? - if [ $rc = 5 ]; then - try_trace "$RTLD" "$file" - rc=$? - fi - [ $rc = 0 ] || result=1 - ;; 1) # This can be a non-ELF binary or no binary at all. nonelf "$file" || { @@ -183,7 +169,7 @@ warning: you do not have execution permission for" "\`$file'" >&2 result=1 } ;; - 2) + [02]) try_trace "$RTLD" "$file" || result=1 ;; *)