-
Notifications
You must be signed in to change notification settings - Fork 245
Prevent DoS attack #213
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
Prevent DoS attack #213
Conversation
A pathname containing `\u0000 NULL` will crash `fs.stat` with the error message "TypeError [ERR_INVALID_ARG_VALUE]: The argument 'path' must be a string or Uint8Array without null bytes." This commit prevents node-static from crashing.
shouldn't this be related to / fixed in the specific Node runtime in use? |
if (pathname.indexOf(that.root) === 0) { | ||
// file outside of the root or an invalid | ||
// pathname. | ||
if (pathname.indexOf(that.root) === 0 && pathname.indexOf('\u0000') === -1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you would do this like this, it should be somehow at least logging that this type of path has been ignored i think.
is there some documentation on this vulnerability and have there been other fixes to this exploit in other open-source codebases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't log paths like ../
either; I just modeled my code after the existing code.
I'm not aware of documentation of this problem. I became aware of it because someone attacked my server with it.
It looks like this is new in Node.js v10: https://alexatnet.com/node-js-10-important-changes/#fs-2 In previous Node versions, it would be passed as an error to the
But in Node 10 and later, |
is the change in Node.js that led to this regression. |
This is a serious security issue. What is the state of this PR ? |
Why is this still not closed? 🤔 |
Closing as #227 avoids need for this PR. |
A pathname containing
U+0000 NULL
will crashfs.stat
with the error message "TypeError [ERR_INVALID_ARG_VALUE]: The argument 'path' must be a string or Uint8Array without null bytes."This commit prevents node-static from crashing.