3 ms·

How do you arrive at the conclusion that looking at `./plugins/` is bad, but `../shared/dstat` is more acceptable? At first blush, the attack vector would seem

by Kalium 7y ago

How do you arrive at the conclusion that looking at `./plugins/` is bad, but `../shared/dstat` is more acceptable? At first blush, the attack vector would seem to be exactly the same. Adding the extra layer of indirection does not seem, to my eyes, to add any significant extra layers of security. Both seem essentially untrustable from the perspective of the binary. Can you help me understand what I've missed in your explanation? I can also see where someone might not want to endlessly re-debate an old security decision.

lstamour 7y ago
Sure. The original security bug was lines 31-32, but the new patch referring to the parent directory is a variation of line 30, untouched here: https://bugs.gentoo.org/attachment.cgi?id=210509&action=diff https://bugs.gentoo.org/attachment.cgi?id=210509&action=diff CWD is far worse a risk, because it changes every time the command is run. By comparison it could be assumed that a secure install of the software is in a trusted location, and this just gets the parent root of that location. Of course, it’s better if it’s documented, but I see no difference between this and $JAVA_HOME’s approach of ./bin, ./share, etc. It could be argued this is better—no environment variables can override the default. If you install the app to /tmp/bin that’s your own fault...
- Kalium 7y ago
  Ah! Thank you for clarifying. That said, going up a directory would seem to imply that dstat could easily be loading completely arbitrary code from a directory it has no exclusive claim to and thus cannot meaningfully trust. Indeed, as I understand `share/` that is exactly the point. This really doesn't strike me as being a safe thing to do, and I can readily see why the author would reject it on security grounds. This seems to me to be a distinction with little meaningful difference, though I can see where some people might disagree. As I tell PMs, designers, and engineers I work with far too often for my own comfort, someone else's poor security decisions are no excuse for our own.
  - lstamour 7y ago
    If absolute share/ access were the problem, the line below wouldn’t hard-code it twice: https://github.com/dagwieers/dstat/pull/50/files https://github.com/dagwieers/dstat/pull/50/files The only bug in this PR that I can see is it might assume the system plugins are more important than the local overrides. Generally you’d put this sort of path in the least priority, to allow for /usr/local to override. That said, obviously hard-coding paths is worse than simply having a default, presumably relative to the current binary/file, and allowing end users and packagers to override/configure the defaults to suit their security preferences, with the default being that the entire package is unzipped with expected relative paths to files... or that’s how I would look at it. The only way to greater security would be to ship a Docker container, Snap package, or similar and mount your own filesystem overlays. :) Or, perhaps keeping a trusted list of plugins somewhere.
    - Kalium 7y ago
      My read is that it's more that access control of `../share` in an unknown part of the filesystem is a matter for some concern. Given that the binary can be put basically anywhere, it would seem to be perilously close to CWD.
      - contras1970 7y ago
        no, it's just cargocult security. edit after your reply: if you install in /tmp, you'll end up with /tmp/bin/dstat and /tmp/share/dstat. you're concerned that an attacker could smuggle something into /tmp/share/dstat, but /tmp/bin/dstat is of no worry? what exactly is the threat here? > access control of `../share` in an unknown part of the filesystem is a matter for some concern. Given that the binary can be put basically anywhere, it would seem to be perilously close to CWD. another edit since i cannot reply to you: do you have any examples of the "threat model [which] includes that you can't trust every part of the filesystem you're working from"? something concrete, specific. a particular install prefix that would let you create $prefix/bin/dstat but $prefix/share/dstat would be dangerous. aaand, see my reply at https://news.ycombinator.com/item?id=19989237 https://news.ycombinator.com/item?id=19989237

by Kalium 7y ago

lstamour 7y ago

Sure. The original security bug was lines 31-32, but the new patch referring to the parent directory is a variation of line 30, untouched here: https://bugs.gentoo.org/attachment.cgi?id=210509&action=diff https://bugs.gentoo.org/attachment.cgi?id=210509&action=diff CWD is far worse a risk, because it changes every time the command is run. By comparison it could be assumed that a secure install of the software is in a trusted location, and this just gets the parent root of that location. Of course, it’s better if it’s documented, but I see no difference between this and $JAVA_HOME’s approach of ./bin, ./share, etc. It could be argued this is better—no environment variables can override the default. If you install the app to /tmp/bin that’s your own fault...

Kalium 7y ago
Ah! Thank you for clarifying. That said, going up a directory would seem to imply that dstat could easily be loading completely arbitrary code from a directory it has no exclusive claim to and thus cannot meaningfully trust. Indeed, as I understand `share/` that is exactly the point. This really doesn't strike me as being a safe thing to do, and I can readily see why the author would reject it on security grounds. This seems to me to be a distinction with little meaningful difference, though I can see where some people might disagree. As I tell PMs, designers, and engineers I work with far too often for my own comfort, someone else's poor security decisions are no excuse for our own.
- lstamour 7y ago
  If absolute share/ access were the problem, the line below wouldn’t hard-code it twice: https://github.com/dagwieers/dstat/pull/50/files https://github.com/dagwieers/dstat/pull/50/files The only bug in this PR that I can see is it might assume the system plugins are more important than the local overrides. Generally you’d put this sort of path in the least priority, to allow for /usr/local to override. That said, obviously hard-coding paths is worse than simply having a default, presumably relative to the current binary/file, and allowing end users and packagers to override/configure the defaults to suit their security preferences, with the default being that the entire package is unzipped with expected relative paths to files... or that’s how I would look at it. The only way to greater security would be to ship a Docker container, Snap package, or similar and mount your own filesystem overlays. :) Or, perhaps keeping a trusted list of plugins somewhere.
  - Kalium 7y ago
    My read is that it's more that access control of `../share` in an unknown part of the filesystem is a matter for some concern. Given that the binary can be put basically anywhere, it would seem to be perilously close to CWD.
    - contras1970 7y ago
      no, it's just cargocult security. edit after your reply: if you install in /tmp, you'll end up with /tmp/bin/dstat and /tmp/share/dstat. you're concerned that an attacker could smuggle something into /tmp/share/dstat, but /tmp/bin/dstat is of no worry? what exactly is the threat here? > access control of `../share` in an unknown part of the filesystem is a matter for some concern. Given that the binary can be put basically anywhere, it would seem to be perilously close to CWD. another edit since i cannot reply to you: do you have any examples of the "threat model [which] includes that you can't trust every part of the filesystem you're working from"? something concrete, specific. a particular install prefix that would let you create $prefix/bin/dstat but $prefix/share/dstat would be dangerous. aaand, see my reply at https://news.ycombinator.com/item?id=19989237 https://news.ycombinator.com/item?id=19989237