You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Ah. Well, I presume the protection needs to be at the Node.js server level regardless, right? Is this something that Node.js is missing? There are many, many body parsing modules on npm, including the ability to make them yourself of course.
my understanding is that it's the body parser's responsibility to implement the timeout.
Why would that be part of parsing and not part of the stream reading itself? Is this stated somewhere you can link to for where your understanding comes from? If you're saying this is a feature you think should exist here, you're welcome to make a pull request. But if I'm understanding your request correctly that would be a hurge disservice to the npm ecosystem at large as I presume every single module would need to implement this?
There doesn't seem to be any indication that low level timeouts should be the responsibility of the reader and looking at the two "reference" modules linked there they don't seem to be doing any kind of special timeout behavior? Is it really a feature missing from Node.js?
I would suggest contacting Node.js following https://nodejs.org/en/security/ is possible. The issue adding it here is that (1) users may not even use this module (2) use a different module (3) use a different framework based on Node.js. It seems strange they have some timeouts and not others. The inactivity timeout for example is on the body, so it's not like there are no body timeouts currently in Node.js. Perhaps this is an overlooked vector.
Activity
dougwilson commentedon Feb 18, 2020
Yes, it is part of Node.js itself, which is why it is not implemented again in this module: https://nodejs.org/dist/latest-v13.x/docs/api/http.html#http_server_timeout
underflow00 commentedon Feb 18, 2020
That is an inactivity timeout. It does not apply here.
dougwilson commentedon Feb 18, 2020
Ah. Well, I presume the protection needs to be at the Node.js server level regardless, right? Is this something that Node.js is missing? There are many, many body parsing modules on npm, including the ability to make them yourself of course.
dougwilson commentedon Feb 18, 2020
I see Node.js has a headers timeout, at least: https://nodejs.org/dist/latest-v13.x/docs/api/http.html#http_server_headerstimeout
underflow00 commentedon Feb 18, 2020
Yes, my understanding is that it's the body parser's responsibility to implement the timeout (of parsing the body).
dougwilson commentedon Feb 18, 2020
Why would that be part of parsing and not part of the stream reading itself? Is this stated somewhere you can link to for where your understanding comes from? If you're saying this is a feature you think should exist here, you're welcome to make a pull request. But if I'm understanding your request correctly that would be a hurge disservice to the npm ecosystem at large as I presume every single module would need to implement this?
dougwilson commentedon Feb 18, 2020
This is the general docs I refer to when looking for the body reading Node.js docs: https://nodejs.org/en/docs/guides/anatomy-of-an-http-transaction/#request-body
There doesn't seem to be any indication that low level timeouts should be the responsibility of the reader and looking at the two "reference" modules linked there they don't seem to be doing any kind of special timeout behavior? Is it really a feature missing from Node.js?
underflow00 commentedon Feb 18, 2020
I'm going to go with yes.
There are few socket related Node.js issues open for years so it's not surprising.
Relevant issues:
nodejs/node#6286
nodejs/node#3319
The attack can easily be re-created:
redacted
dougwilson commentedon Feb 18, 2020
I would suggest contacting Node.js following https://nodejs.org/en/security/ is possible. The issue adding it here is that (1) users may not even use this module (2) use a different module (3) use a different framework based on Node.js. It seems strange they have some timeouts and not others. The inactivity timeout for example is on the body, so it's not like there are no body timeouts currently in Node.js. Perhaps this is an overlooked vector.
underflow00 commentedon Sep 17, 2020
This has been fixed in Node.js's latest security release.