Skip to content

doc: update threat model based on discussions - #46373

Closed
mhdawson wants to merge 1 commit into
nodejs:mainfrom
mhdawson:tweak-threat-model
Closed

doc: update threat model based on discussions#46373
mhdawson wants to merge 1 commit into
nodejs:mainfrom
mhdawson:tweak-threat-model

Conversation

@mhdawson

Copy link
Copy Markdown
Member

Signed-off-by: Michael Dawson mdawson@devrus.com

Signed-off-by: Michael Dawson <mdawson@devrus.com>
@nodejs-github-botnodejs-github-bot added the doc Issues and PRs related to the documentations. label Jan 26, 2023
@mhdawson

Copy link
Copy Markdown
MemberAuthor

@RafaelGSS, @mcollina, @jasnell how does this look to you?

@panvapanva changed the title doc: update thread model based on discussionsdoc: update threat model based on discussionsJan 26, 2023

@RafaelGSSRafaelGSS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

Comment threadSECURITY.md
Comment on lines +194 to +197
* If Node.js is asked to connect to a remote site and return an
artifact, it is not considered a vulnerability if the size of
that artifact is large enough to impact performance and or
cause the runtime to run out of resources.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this actually true? Say I host a malicious server that triggers a hash table collision denial-of-service in node's http client - that's considered a vulnerability, right?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@bnoordhuis I think your example is different in that it does not relate to the size of the artifact returned.

mhdawson added a commit that referenced this pull request Feb 1, 2023
Signed-off-by: Michael Dawson <mdawson@devrus.com>
PR-URL: #46373
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@mhdawson

Copy link
Copy Markdown
MemberAuthor

Landed in 088e470

@mhdawsonmhdawson closed this Feb 1, 2023
MylesBorins pushed a commit that referenced this pull request Feb 18, 2023
Signed-off-by: Michael Dawson <mdawson@devrus.com>
PR-URL: #46373
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@MylesBorinsMylesBorins mentioned this pull request Feb 19, 2023
danielleadams pushed a commit that referenced this pull request Apr 11, 2023
Signed-off-by: Michael Dawson <mdawson@devrus.com>
PR-URL: #46373
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docIssues and PRs related to the documentations.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@mhdawson@mcollina@bnoordhuis@anonrig@RafaelGSS@nodejs-github-bot