Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security Fix for Directory Traversal - huntr.dev #6

Merged
merged 6 commits into from Sep 16, 2020

Conversation

huntr-helper
Copy link

https://huntr.dev/users/Mik317 has fixed the Directory Traversal vulnerability 🔨. Mik317 has been awarded $25 for fixing the vulnerability through the huntr bug bounty program 💵. Think you could fix a vulnerability like this?

Get involved at https://huntr.dev/

Q | A
Version Affected | ALL
Bug Fix | YES
Original Pull Request | 418sec#1
Vulnerability README | https://github.com/418sec/huntr/blob/master/bounties/npm/web-node-server/1/README.md

User Comments:

Bounty URL: https://www.huntr.dev/bounties/1-npm-web-node-server

⚙️ Description *

The nodeserver server was vulnerable against path traversal through the ../ technique, leading to information exposure and file content disclosure.

💻 Technical Description *

I implemented a simple fix which makes possible sanitizing any occurrence of ../and different vectors, using a regex, in order to check and avoid bypasses OS-based which are really common nowadays.
I didn't add other pages for forbidden message, since when a file is sanitized and not found, by design the server will reply with the index.html file.

🐛 Proof of Concept (PoC) *

  1. Download the original server
  2. Create the file test.js with the following content:
var config = {
    'localhost': {
        backend: __dirname + '/',
        frondend: __dirname + '/',
        baseTemp: 'index.html'
    }
};
var pkg = require('./nodeserver/nodeserver');
pkg.start(config);
  1. Start the server: node test.js
  2. PoC: curl --path-as-is http://localhost:9999/../../../../../../../../../../etc/passwd
  3. Content of /etc/passwd is showed 😄

Screenshot from 2020-08-22 00-54-28

🔥 Proof of Fix (PoF) *

Same steps above with fixed version == no /etc/passwd showed 😄

Screenshot from 2020-08-22 01-10-38

👍 User Acceptance Testing (UAT)

All OK 😄

Mik317 and others added 6 commits August 22, 2020 01:12
* Added a better regex to avoid breaking files named `..valid`
* Added path normalization to avoid OS-based bypasses
Removed the `path.normalize()` call for `security reason` since it's the vector of another class of `path traversal`
[FIX] Path traversal sanitizing request path
@youngerheart youngerheart merged commit 4a2828f into youngerheart:master Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants