bugfindutils - Bugs: bug #18576, -execdir vs. PATH

 
 

bug #18576: -execdir vs. PATH

Submitter:  Eric Blake <ericb>
Submitted:  Fri 22 Dec 2006 01:39:03 PM UTC
   
 
Category:  find Severity:  1 - Wish
Item Group:  Wrong result Status:  Wont Fix
Privacy:  Public Assigned to:  jay
Originator Name:  Eric Blake Open/Closed:  Closed
Release:  4.3.2 Fixed Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 15 Apr 2007 07:48:08 PM UTC, comment #10: 

Discussion seems to have petered out on this bug, which I interpret to mean that people are reasonably heppy with the outcome.   Please open a new bug if there are remaining problems.

James Youngman <jay>
Group administrator
Sun 07 Jan 2007 06:58:51 PM UTC, comment #9: 

After a bit of thought I've decided to leave the functionality as it is, for a number of reasons, none of which is by itself very influential:

1. It's fairly easy to work around the current situation if you don't like it (e.g. by unsetting PATH).

2. Relative directories on the path are almost always a bad idea, so I'm not very persuaded by considerations of making life easier for those with that misconfiguration.

3. The proposed compromise adds complexity to the code and to the documentation. 

4. If find suspects but cannot prove that a relative path search will occur, it can't be certain whether or not something dangerous is going to happen.  If the situation is uncertain I'd prefer to live with a cautious but somewhat irritating choice rather than a convenient but potentially insecure one.

So, while I stated "I agree" on 22 Dec, I changed my mind on the 29th.  I have changed the findutils code in response to this bug report, but not in the way that Eric really wanted, I think.  So it would be a bit disingenuous to mark the bug as "Fixed".  Instead I will mark it as "Won't Fix".

If you feel strongly about this, I would be prepared to apply a patch which allowed the behaviour you prefer to be enabled with some --enable-foo configure option.  If you do decide to go down that route, please be careful to keep the number of alternative code paths down (in order to ensure that the relevant code gets routinely tested and doesn't end up with a bug nobody notices for ages).


James Youngman <jay>
Group administrator
Fri 29 Dec 2006 08:43:41 PM UTC, comment #8: 

True, any command that invokes another app (nice, su, nohup, ...) can perform PATH searches.  But is this really find's problem?  It is not sensible to teach find about every program that invokes one of its arguments as another program.  Maybe a compromise is in order: if PATH contains relative elements, find should always issue a warning, regardless of whether command or its arguments have a /, on the grounds that the invoked command may also cause an insecure PATH search.  Additionally, if command does not contain /, and a PATH search encounters a relative path before finding command, then find should outright fail.  In other words, only fail when find can prove that a relative path search will occur, but warn the user of the security potential without worrying about deciphering the semantics of how command will further parse its arguments.

Eric Blake <ericb>
Group administrator
Fri 29 Dec 2006 12:56:38 PM UTC, comment #7: 

I've updated the code to complain about PATH=foo:/bin but I haven't addressed Eric's original problem because of the difficulty I describe in my previous comment.

James Youngman <jay>
Group administrator
Fri 29 Dec 2006 12:48:57 PM UTC, comment #6: 

Hmm, there is another problem too.   The command may execute binaries by searching $PATH even if the first argument starts with a slash...

PATH=".:/bin:/usr/bin"
find /tmp -execdir /usr/bin/nice wc {} \;


James Youngman <jay>
Group administrator
Wed 27 Dec 2006 02:54:54 AM UTC, comment #5: 

Yes, -execdir currently fails when . is on PATH, and I don't see any reason to change that aspect of the behavior, since -execdir is not standardized.  If you relax to just a warning, then by the time a warning is printed, find would have already forked the problem process.  And if you want interactivity, -okdir fits the bill (although maybe it would be worth adding '-okdir command {} +'?).

Eric Blake <ericb>
Group administrator
Tue 26 Dec 2006 04:37:11 PM UTC, comment #4: 

"[...] then fail due to the security risk." - fail, really? Not just warn?

Egmont Koblinger <egmont>
Fri 22 Dec 2006 09:40:10 PM UTC, comment #3: 

Useful idea, thanks.   There's a race condition there, but there is certainly scope to reduce the number of false positives, as you say.

(OK, removal of /bin/echo may not be a realistic race condition, but in other directories it may be a slightly more reasonable fear)

James Youngman <jay>
Group administrator
Fri 22 Dec 2006 09:15:59 PM UTC, comment #2: 

One additional reduction in false positives is still possible.  Currently, -execdir gripes even if the PATH search would not have found the directory-relative program:

$ PATH=/usr/bin find -execdir echo {} +
./a
$ PATH=/usr/bin: find -execdir echo {} +
find: The current directory is included in the PATH environment variable, which is insecure in combination with the -execdir action of find.  Please remove the current directory from your $PATH (that is, remove "." or leading or trailing colons)

But it should be possible to examine the PATH, and realize that since /usr/bin/echo exists, it will always be chosen rather than ./echo.  A reformulation of this is that from a security point of view, /bin:. is a more secure PATH than .:/bin (both have risk, but by listing . last, you have reduced the risk; no matter what directory you are in, you will still get /bin/echo).

So, maybe the rule should be:
If the first argument after -execdir contains /, proceed without warning.
Otherwise, start visiting components in PATH, looking for an executable file that matches the first argument after -execdir.  If the command is found while encountering only absolute path components, proceed without warning.  But if any path component is found that is relative, and the program has not yet been discovered, then fail due to the security risk.

Eric Blake <ericb>
Group administrator
Fri 22 Dec 2006 05:01:09 PM UTC, comment #1: 

I agree.

James Youngman <jay>
Group administrator
Fri 22 Dec 2006 01:39:03 PM UTC, original submission:  

-execdir has some annoying behavior:

$ PATH=:$PATH find -execdir /bin/echo {} +
find: The current directory is included in the PATH environment variable, which is insecure in combination with the -execdir action of find.  Please remove the current directory from your $PATH (that is, remove "." or leading or trailing colons)

Here, I don't think the security warning should occur.  Because my command (/bin/echo) is absolute, it should not matter what PATH contains, because PATH is not consulted in anything invoked by find.

On the other hand, -execdir should warn on this, but currently doesn't:
$ mkdir a
$ cat > a/echo << EOF
#!/bin/sh
echo wrong echo
EOF
$ chmod +x a/echo
$ PATH=./.:$PATH find -execdir echo {} +
wrong echo

The key here is that it is not just the presence of '.' in PATH, but the presence of ANY element in PATH that does not start with /, where a path lookup can result in executing a different executable depending on the current directory at the time of the path lookup.

Which means I also think that find should not warn on this case, again because the presence of / in the command means that PATH is not consulted, and I really DID intend to execute my echo, because I just validated with -name that it exists:

$ mkdir a
$ cat > a/echo << EOF
#!/bin/sh
echo my echo
EOF
$ chmod +x a/echo
$ find -name -execdir ./echo {} +
my echo
$ PATH=:$PATH find -name -execdir ./echo {} +
find: The current directory is included in the PATH environment variable, which is insecure in combination with the -execdir action of find.  Please remove the current directory from your $PATH (that is, remove "." or leading or trailing colons)

In short, I think the rule should be:
If the first argument after -execdir contains no /, and if any component of PATH does not start with /, warn.
Otherwise, no PATH lookup will occur, so proceed, even if PATH contains a relative component or if command is relative.

Eric Blake <ericb>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by egmont (Posted a comment)
  • -email is unavailable- added by jay (Posted a comment)
  • -email is unavailable- added by ericb (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-04-15 jay Assigned toNone jay
        Open/ClosedOpen Closed
    2007-01-07 jay Severity3 - Normal 1 - Wish
        StatusNone Wont Fix

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code