Skip to content
New issue

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

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

Already on GitHub? # to your account

NodeGoat Research page SSRF also enables DoS #225

Closed
rcowsill opened this issue Jan 24, 2021 · 1 comment
Closed

NodeGoat Research page SSRF also enables DoS #225

rcowsill opened this issue Jan 24, 2021 · 1 comment

Comments

@rcowsill
Copy link
Contributor

The research page intentionally exposes a SSRF vulnerability which allows an attacker to submit unexpected url and query params. The tutorial demonstrates how this can be used to make the server connect to an attacker-controlled site.

As well as this expected vulnerability there's a bug in app/routes/research.js which crashes the server if the request to that target URL fails.

This happens when visiting (eg) http://localhost:4000/research?url=localhost%3A4001%2F&symbol=login while the server is running locally on port 4000:

[...]\app\routes\research.js:23
                res.write(newResponse.body);
                                      ^

TypeError: Cannot read property 'body' of undefined
    at [...]\app\routes\research.js:23:39
    at done ([...]\node_modules\needle\lib\needle.js:442:14)
    at ClientRequest.had_error ([...]\node_modules\needle\lib\needle.js:452:5)
    at ClientRequest.emit (events.js:310:20)
    at Socket.socketErrorListener (_http_client.js:426:9)
    at Socket.emit (events.js:310:20)
    at emitErrorNT (internal/streams/destroy.js:92:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
    at processTicksAndRejections (internal/process/task_queues.js:84:21)

What's happening is that the needle callback skips setting the content-type when there's an error, but still tries to append newResponse.body to res. If there's an error, newResponse is undefined and the server crashes.

@ckarande
Copy link
Member

@rcowsill Thanks for reporting this issue and identifying the root cause. Yes, there should be a check for undefined value of newResponse. I'd be happy to merge a PR in case you have bandwidth to fix it.

rcowsill added a commit to rcowsill/NodeGoat that referenced this issue Jan 25, 2021
In addition to the intended SSRF vulnerability, it was possible to
crash the server with maliciously chosen query parameters.

Closes OWASP#225
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants