7 ms·

> File-system Based Routing: If your code resides in ./api/login.js it's exposed at http://<SERVER>/api/login http://<SERVER>/api/login. Inspired by good ol' PH

by q3k 8y ago

> File-system Based Routing: If your code resides in ./api/login.js it's exposed at http://<SERVER>/api/login http://<SERVER>/api/login. Inspired by good ol' PHP days. > Auto Dependency Resolution: If a file does require('underscore'), it is automatically installed and resolved. You can always create your own package.json file to install a specific version of a package. This sounds like a security nightmare. EDIT: to be clear, I don't meant he combination of both is a security concern, that each one of them separately is problematic.

gpmcadam 8y ago
Just like "good ol' PHP days!" as the docs say :D
untog 8y ago
...why? I get that file-system based routing means you know the location of a source file on disk, but if anyone can access that file you've already lost. And auto-dependency resolution also doesn't seem any larger a security concern, all it's doing is skipping an "npm install" command.
- q3k 8y ago
  Because if you ever have a broken upload system that allows you to drop a JS file somewhere accessible by the file system routing, you have remote code execution. Additionally, you now have to write guards in every non-endpoint JS file so that it doesn't get executed just by a misplaced HTTP request. And as for automatic dependency resolution, this means you're not even aware of what transitive dependencies you're pulling in, what version they are and have no way to vet anything - everything is hidden behind a wall of magic.
  - adrusi 8y ago
    Make your application directory read-only to the user running the application, as it ought to be anyway. Automatic dependency resolution however... Fantastic for experimentation, but that's a dealbreaker for production. Maybe it would be OK if it actually wrote the package-lock.json to the application directory, I'd have to think about that.
    - SahAssar 8y ago
      In this case the application also tries to auto-install dependencies, so making it read-only removes one of the stated features. I think this framework hasn't been written with security in mind at all.
      - adrusi 8y ago
        Whatever it's doing, it's not writing the packages into the application directory.
        SahAssar 8y ago
        If it can write to the applications dependencies isn't that as good as writing to the application?
- aboutruby 8y ago
  Probably having .htaccess / .env / database configuration / files that are not supposed to be public be exposed. For instance, Rails has a public/ folder for files that are going to be served. And jekyll hides files by pattern-matching them[1]. Zero doesn't seem to have exclude folders by default. The solution would be to run Zero is a subfoler and require file in the parent folder which would act as the tree's root. [1]: https://help.github.com/en/articles/files-that-start-with-an-underscore-are-missing https://help.github.com/en/articles/files-that-start-with-an...
  - asadlionpk 8y ago
    Currently, files starting with _ (underscore) are hidden in zero. This is still a feature spec we need to finalize as this can create confusion. Maybe a .zeroignore file (as suggested in another comment) would be a better idea.
    - curioussavage 8y ago
      Why not reverse that and use a whitelist instead. It’s a lot easier to decide what folders and files should be served than to think of all the things that shouldn’t.
      - deleted 8y ago
        [deleted]
    - scrollaway 8y ago
      Nextjs uses a pages/ subdirectory which gives you implicit routing, without having to compromise on the whitelist aspect. I think it's a better compromise.
jayar95 8y ago
If security is a concern, this is probably a bad choice; this doesn't seem to be advertised as a bulletproof security solution to anything, rather a utility for small little one-off apps that might need _some_ backend functionality. Once you start adding features like file-uploading, youre obviously gonna want to pick a more robust option
- beefhash 8y ago
  > If security is a concern At the risk of being presumptuous... When is security ever not a concern?
  - reidjs 8y ago
    Student projects
    - lsiunsuex 8y ago
      Conversely though - doesn't that lead to bad habits? If security is taught at the student level, by the time they get to junior developer they'll have an understanding of it / do it automatically.
    - q3k 8y ago
      When you're learning, you need to also learn security implications of what you're writing. Insecure projects should never be allowed to pass.
      - reidjs 8y ago
        I disagree somewhat. If the goal of a project is to teach a different skill and it may cause too much of a headache to add a real server this service might make sense. It’s like when you’re learning a new spoken language. It’s better to practice a breadth of situations and vocabularies and make mistakes (that get corrected over time) than to learn fewer things perfectly
    - wesleytodd 8y ago
nailer 8y ago
So if an attacker already has access to your filesystem to modify your files, they can install stuff? By this time it's already too late.
- q3k 8y ago
  There's different levels of 'filesystem access' vulnerabilities. Some classes of bugs that would be otherwise tame due to the constraints (eg., file upload that might be able to only create new files in some part of the directory tree, or a buggy routine that lets you create arbitrary symlinks, or leftover VCS/CM files that happen to end in .js and are not filtered out by the router) now become the most powerful kind, remote code execution.
  - nailer 8y ago
    Exploiting auto-downloading modules requires that the filesystem's already been exploited to the point where the app's code can be modified. I could add `require('foo')` but I could also just require no third party code and have fun with the `process` module.
Cu3PO42 8y ago
I don't disagree that this doesn't seem necessarily secure and the auto dependency resolution is a bad idea for other reasons in my opinion, but I don't see the security aspect of it. The moment I can upload files to the application folder that are executed, I can just `require('child_process').spawn("my_evil_stuff", [])`. In particular "my_evil_stuff" could be some npm install command. I don't see how automatically installing the dependencies makes this worse than it already is. EDIT: While this is a different attack vector than I was envisioning here, jexco has provided a scenario in which there are additional vulnerabilities: Vulnerable dependencies that cannot be managed.
- jecxjo 8y ago
  Where is the management of requirements? How do you force LTS versions? Or roll back if a version has a vulnerability? When things are automatic you are unable to stop bad things from happening.
  - Cu3PO42 8y ago
    Not that I was going to say this is a good idea in the first place, but rolling back vulnerable dependencies is an excellent point that I hadn't thought of. Thanks!
- SahAssar 8y ago
  Well, for one giving the app the kinds of write permissions needed for this to work is not exactly ideal.
  - Cu3PO42 8y ago
    So the app would need write permissions to its own folder. That's obviously a bad idea in a production deployment. I guess I was thinking that the dependencies would be installed during a privileged one-time "deployment run" so you wouldn't need the permissions after. Maybe I'm giving the thing too much credit.
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

by q3k 8y ago

gpmcadam 8y ago

Just like "good ol' PHP days!" as the docs say :D

untog 8y ago

...why? I get that file-system based routing means you know the location of a source file on disk, but if anyone can access that file you've already lost. And auto-dependency resolution also doesn't seem any larger a security concern, all it's doing is skipping an "npm install" command.

q3k 8y ago
Because if you ever have a broken upload system that allows you to drop a JS file somewhere accessible by the file system routing, you have remote code execution. Additionally, you now have to write guards in every non-endpoint JS file so that it doesn't get executed just by a misplaced HTTP request. And as for automatic dependency resolution, this means you're not even aware of what transitive dependencies you're pulling in, what version they are and have no way to vet anything - everything is hidden behind a wall of magic.
- adrusi 8y ago
  Make your application directory read-only to the user running the application, as it ought to be anyway. Automatic dependency resolution however... Fantastic for experimentation, but that's a dealbreaker for production. Maybe it would be OK if it actually wrote the package-lock.json to the application directory, I'd have to think about that.
  - SahAssar 8y ago
    In this case the application also tries to auto-install dependencies, so making it read-only removes one of the stated features. I think this framework hasn't been written with security in mind at all.
    - adrusi 8y ago
      Whatever it's doing, it's not writing the packages into the application directory.
      - SahAssar 8y ago
        If it can write to the applications dependencies isn't that as good as writing to the application?
aboutruby 8y ago
Probably having .htaccess / .env / database configuration / files that are not supposed to be public be exposed. For instance, Rails has a public/ folder for files that are going to be served. And jekyll hides files by pattern-matching them[1]. Zero doesn't seem to have exclude folders by default. The solution would be to run Zero is a subfoler and require file in the parent folder which would act as the tree's root. [1]: https://help.github.com/en/articles/files-that-start-with-an-underscore-are-missing https://help.github.com/en/articles/files-that-start-with-an...
- asadlionpk 8y ago
  Currently, files starting with _ (underscore) are hidden in zero. This is still a feature spec we need to finalize as this can create confusion. Maybe a .zeroignore file (as suggested in another comment) would be a better idea.
  - curioussavage 8y ago
    Why not reverse that and use a whitelist instead. It’s a lot easier to decide what folders and files should be served than to think of all the things that shouldn’t.
    - deleted 8y ago
      [deleted]
  - scrollaway 8y ago
    Nextjs uses a pages/ subdirectory which gives you implicit routing, without having to compromise on the whitelist aspect. I think it's a better compromise.

jayar95 8y ago

If security is a concern, this is probably a bad choice; this doesn't seem to be advertised as a bulletproof security solution to anything, rather a utility for small little one-off apps that might need _some_ backend functionality. Once you start adding features like file-uploading, youre obviously gonna want to pick a more robust option

beefhash 8y ago
> If security is a concern At the risk of being presumptuous... When is security ever not a concern?
- reidjs 8y ago
  Student projects
  - lsiunsuex 8y ago
    Conversely though - doesn't that lead to bad habits? If security is taught at the student level, by the time they get to junior developer they'll have an understanding of it / do it automatically.
  - q3k 8y ago
    When you're learning, you need to also learn security implications of what you're writing. Insecure projects should never be allowed to pass.
    - reidjs 8y ago
      I disagree somewhat. If the goal of a project is to teach a different skill and it may cause too much of a headache to add a real server this service might make sense. It’s like when you’re learning a new spoken language. It’s better to practice a breadth of situations and vocabularies and make mistakes (that get corrected over time) than to learn fewer things perfectly
  - wesleytodd 8y ago

nailer 8y ago

So if an attacker already has access to your filesystem to modify your files, they can install stuff? By this time it's already too late.

q3k 8y ago
There's different levels of 'filesystem access' vulnerabilities. Some classes of bugs that would be otherwise tame due to the constraints (eg., file upload that might be able to only create new files in some part of the directory tree, or a buggy routine that lets you create arbitrary symlinks, or leftover VCS/CM files that happen to end in .js and are not filtered out by the router) now become the most powerful kind, remote code execution.
- nailer 8y ago
  Exploiting auto-downloading modules requires that the filesystem's already been exploited to the point where the app's code can be modified. I could add `require('foo')` but I could also just require no third party code and have fun with the `process` module.

Cu3PO42 8y ago

I don't disagree that this doesn't seem necessarily secure and the auto dependency resolution is a bad idea for other reasons in my opinion, but I don't see the security aspect of it. The moment I can upload files to the application folder that are executed, I can just `require('child_process').spawn("my_evil_stuff", [])`. In particular "my_evil_stuff" could be some npm install command. I don't see how automatically installing the dependencies makes this worse than it already is. EDIT: While this is a different attack vector than I was envisioning here, jexco has provided a scenario in which there are additional vulnerabilities: Vulnerable dependencies that cannot be managed.

jecxjo 8y ago
Where is the management of requirements? How do you force LTS versions? Or roll back if a version has a vulnerability? When things are automatic you are unable to stop bad things from happening.
- Cu3PO42 8y ago
  Not that I was going to say this is a good idea in the first place, but rolling back vulnerable dependencies is an excellent point that I hadn't thought of. Thanks!
SahAssar 8y ago
Well, for one giving the app the kinds of write permissions needed for this to work is not exactly ideal.
- Cu3PO42 8y ago
  So the app would need write permissions to its own folder. That's obviously a bad idea in a production deployment. I guess I was thinking that the dependencies would be installed during a privileged one-time "deployment run" so you wouldn't need the permissions after. Maybe I'm giving the thing too much credit.

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