Skip to content

JS: add http request server data as a RemoteFlowSource#4338

Merged
codeql-ci merged 1 commit intogithub:mainfrom
erik-krogh:nodejs-server-request-data
Oct 1, 2020
Merged

JS: add http request server data as a RemoteFlowSource#4338
codeql-ci merged 1 commit intogithub:mainfrom
erik-krogh:nodejs-server-request-data

Conversation

@erik-krogh
Copy link
Contributor

@erik-krogh erik-krogh commented Sep 24, 2020

Inspired by a synthetic benchmark.
We don't gain any results on that benchmark, but we are one step closer.

@github-actions github-actions bot added the JS label Sep 24, 2020
@erik-krogh erik-krogh force-pushed the nodejs-server-request-data branch 2 times, most recently from 3ba6e54 to 27fbd70 Compare September 28, 2020 18:52
@erik-krogh erik-krogh marked this pull request as ready for review September 29, 2020 15:10
@erik-krogh erik-krogh requested a review from a team as a code owner September 29, 2020 15:10
* E.g. `chunk` in: `http.createServer().on('request', (req, res) => req.on("data", (chunk) => ...))`.
*/
private class ServerRequestDataEvent extends RemoteFlowSource, DataFlow::ParameterNode {
RequestSource req;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest doing this with HTTP::RequestSource rather than the NodeJS-specific one. Some web frameworks, like Express, exposes request objects that are actually Node.js request objects with a modified prototype.

@erik-krogh erik-krogh force-pushed the nodejs-server-request-data branch from 27fbd70 to 4dec217 Compare October 1, 2020 11:22
Copy link
Contributor

@asgerf asgerf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@codeql-ci codeql-ci merged commit 36450a8 into github:main Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants