3 ms·

This isn't relative to the binary install, it's relative to argv[0]. It's the same security issue -- that argument is under user control. You can make a symlink

This isn't relative to the binary install, it's relative to argv[0]. It's the same security issue -- that argument is under user control. You can make a symlink to your target wherever you want and then argv[0] will be the symlink location. I don't think there is any secure way to get the current script path in Python.

contras1970 7y ago
so you install dstat in /usr/bin/dstat + /usr/share/dstat (because you are root), and an attacker creates /home/eve/bin/dstat with /home/eve/share/dstat/evil.py. why would you run /home/eve/bin/dstat? if eve can get you to run dstat from here ~/bin, why wouldn't she just have ~/bin/dstat with completely different contents? i'm still convinced this is cargocult security.
- mehrdadn 7y ago
  You're right that this wouldn't matter if the program is run with the same privileges as the caller. I was imagining if that wasn't the case (e.g. the user gives dstat setuid permissions), then letting it load arbitrary code could produce a security hole. Admittedly I didn't think too hard about whether/why something like this might be done -- if it wouldn't be, then never mind. As a matter of general safe practice I err on the side of caution, i.e. not depending on argv[0] to have any particular value for the correctness of the program, because otherwise you have to think through all the possible attack/error scenarios and likely document them for the user, etc... and I thought in any case it was worth pointing out that argv[0] did not necessarily correspond to the program location regardless.
- mehrdadn 7y ago
  Edit/Update: You're right; I'm wrong -- I don't think this is the concern. Namely, I completely missed that the bug was already there in another line, and that the home directory was also being scanned. So it can't be a privilege escalation issue. Moreover I forgot that setuid doesn't work for scripts, so that shouldn't be the issue either. Not sure what's going on now either; I'm confused too.
lstamour 7y ago
Would something based on __file__ be more preferred, then? If so, it could be argued the bug already existed and this patch simply uses the existing behaviour...
- mehrdadn 7y ago
  I don't think __file__ would be any different... it's not even an absolute path necessarily. That's why I said I don't think there's any secure way to do this in Python. You're right though, I didn't see the issue was still there. The home directory should also not be there either. I think I looked at this too hastily but now I'm confused too.
  - user5994461 7y ago
    os.path.realpath(__file__) That will give you the absolute path of the current python script, dereferencing any symlink and the .. notation.
    - mehrdadn 7y ago
      It will also be insecure due to a race condition.

contras1970 7y ago

so you install dstat in /usr/bin/dstat + /usr/share/dstat (because you are root), and an attacker creates /home/eve/bin/dstat with /home/eve/share/dstat/evil.py. why would you run /home/eve/bin/dstat? if eve can get you to run dstat from here ~/bin, why wouldn't she just have ~/bin/dstat with completely different contents? i'm still convinced this is cargocult security.

mehrdadn 7y ago
You're right that this wouldn't matter if the program is run with the same privileges as the caller. I was imagining if that wasn't the case (e.g. the user gives dstat setuid permissions), then letting it load arbitrary code could produce a security hole. Admittedly I didn't think too hard about whether/why something like this might be done -- if it wouldn't be, then never mind. As a matter of general safe practice I err on the side of caution, i.e. not depending on argv[0] to have any particular value for the correctness of the program, because otherwise you have to think through all the possible attack/error scenarios and likely document them for the user, etc... and I thought in any case it was worth pointing out that argv[0] did not necessarily correspond to the program location regardless.
mehrdadn 7y ago
Edit/Update: You're right; I'm wrong -- I don't think this is the concern. Namely, I completely missed that the bug was already there in another line, and that the home directory was also being scanned. So it can't be a privilege escalation issue. Moreover I forgot that setuid doesn't work for scripts, so that shouldn't be the issue either. Not sure what's going on now either; I'm confused too.

lstamour 7y ago

Would something based on __file__ be more preferred, then? If so, it could be argued the bug already existed and this patch simply uses the existing behaviour...

mehrdadn 7y ago
I don't think __file__ would be any different... it's not even an absolute path necessarily. That's why I said I don't think there's any secure way to do this in Python. You're right though, I didn't see the issue was still there. The home directory should also not be there either. I think I looked at this too hastily but now I'm confused too.
- user5994461 7y ago
  os.path.realpath(__file__) That will give you the absolute path of the current python script, dereferencing any symlink and the .. notation.
  - mehrdadn 7y ago
    It will also be insecure due to a race condition.