-
Notifications
You must be signed in to change notification settings - Fork 50
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
Denial of Service in newRequest #159
Comments
Hi @y0d3n Thanks for the report! |
#160 will also need to be considered together. |
At the moment, I would like to finalise the following changes. |
Thanks for the quick fix. |
Hi @y0d3n I'm creating the report to request the CVE number for this issue. |
I've published the report and requested the CVE number: |
[](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [@hono/node-server](https://togithub.com/honojs/node-server) | [`1.10.0` -> `1.10.1`](https://renovatebot.com/diffs/npm/@hono%2fnode-server/1.10.0/1.10.1) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | ### GitHub Vulnerability Alerts #### [CVE-2024-32652](https://togithub.com/honojs/node-server/security/advisories/GHSA-hgxw-5xg3-69jx) ### Impact The application hangs when receiving a Host header with a value that `@hono/node-server` can't handle well. Invalid values are those that cannot be parsed by the `URL` as a hostname such as an empty string, slashes `/`, and other strings. For example, if you have a simple application: ```ts import { serve } from '@​hono/node-server' import { Hono } from 'hono' const app = new Hono() app.get('/', (c) => c.text('Hello')) serve(app) ``` Sending a request with a Host header with an empty value to it: ``` curl localhost:3000/ -H "Host: " ``` The results: ``` node:internal/url:775 this.#updateContext(bindingUrl.parse(input, base)); ^ TypeError: Invalid URL at new URL (node:internal/url:775:36) at newRequest (/Users/yusuke/work/h/159/node_modules/@​hono/node-server/dist/index.js:137:17) at Server.<anonymous> (/Users/yusuke/work/h/159/node_modules/@​hono/node-server/dist/index.js:399:17) at Server.emit (node:events:514:28) at Server.emit (node:domain:488:12) at parserOnIncoming (node:_http_server:1143:12) at HTTPParser.parserOnHeadersComplete (node:_http_common:119:17) { code: 'ERR_INVALID_URL', input: 'http:///' } ``` ### Patches The version `1.10.1` includes the fix for this issue. But, you should use `1.11.0`, which has other fixes related to this issue. [https://github.com/honojs/node-server/issues/160](https://togithub.com/honojs/node-server/issues/160) [https://github.com/honojs/node-server/issues/161](https://togithub.com/honojs/node-server/issues/161) ### Workarounds Nothing. Upgrade your `@hono/node-server`. ### References [https://github.com/honojs/node-server/issues/159](https://togithub.com/honojs/node-server/issues/159) --- ### Release Notes <details> <summary>honojs/node-server (@​hono/node-server)</summary> ### [`v1.10.1`](https://togithub.com/honojs/node-server/releases/tag/v1.10.1) [Compare Source](https://togithub.com/honojs/node-server/compare/v1.10.0...v1.10.1) #### What's Changed - fix: catch ERR_INVALID_URL error in listener by [@​usualoma](https://togithub.com/usualoma) in [https://github.com/honojs/node-server/pull/162](https://togithub.com/honojs/node-server/pull/162) **Full Changelog**: honojs/node-server@v1.10.0...v1.10.1 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "" in timezone America/Chicago, Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/autoblocksai/cli). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zMDEuNCIsInVwZGF0ZWRJblZlciI6IjM3LjMwMS40IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Oh! I am very happy😆 |
Hi, Denial of Service vulnerability discovered.
PoC
Source
Request
curl localhost:3000/ -H "Host: "
As a result, the application stops.
In this case, the argument passed to
new URL
will behttp:///
.node-server/src/request.ts
Lines 117 to 126 in c22f750
Suggested fix
It is recommended to validate the Host header.
In addition, it is possible to manipulate the
c.req.path
by including/
in the Host header.This has not so far led to any direct damage, but it could be used for WAF bypassing, etc., so it is recommended to add this to the validation process.
Source
Request
curl localhost:3000/aaa -H "Host: example.com/bbb?"
Response
The text was updated successfully, but these errors were encountered: