-
Notifications
You must be signed in to change notification settings - Fork 30k
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
doc: add Node.js Threat Model #45223
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,6 +53,132 @@ Here is the security disclosure policy for Node.js | |
the release process above to ensure that the disclosure is handled in a | ||
consistent manner. | ||
|
||
## The Node.js threat model | ||
|
||
In the Node.js threat model, there are trusted elements such as the | ||
underlying operating system. Vulnerabilities that require the compromise | ||
of these trusted elements are outside the scope of the Node.js threat | ||
model. | ||
|
||
For a vulnerability to be eligible for a bug bounty, it must be a | ||
vulnerability in the context of the Node.js threat model. In other | ||
words, it cannot assume that a trusted element (such as the operating | ||
system) has been compromised. | ||
|
||
Being able to cause the following through control of the elements that Node.js | ||
does not trust is considered a vulnerability: | ||
|
||
* Disclosure or loss of integrity or confidentiality of data protected through | ||
the correct use of Node.js APIs. | ||
* The unavailability of the runtime, including the unbounded degradation of its | ||
performance. | ||
|
||
If Node.js loads configuration files or runs code by default (without a | ||
specific request from the user), and this is not documented, it is considered a | ||
vulnerability. | ||
Vulnerabilities related to this case may be fixed by a documentation update. | ||
|
||
**Node.js does NOT trust**: | ||
|
||
1. The data from network connections that are created through the use of Node.js | ||
gireeshpunathil marked this conversation as resolved.
Show resolved
Hide resolved
|
||
APIs and which is transformed/validated by Node.js before being passed to the | ||
application. This includes: | ||
* HTTP APIs (all flavors) client and server APIs. | ||
* DNS APIs. | ||
2. Consumers of data protected through the use of Node.js APIs (for example | ||
people who have access to data encrypted through the Node.js crypto APIs). | ||
3. The file content or other I/O that is opened for reading or writing by the | ||
use of Node.js APIs (ex: stdin, stdout, stderr). | ||
|
||
In other words, if the data passing through Node.js to/from the application | ||
can trigger actions other than those documented for the APIs, there is likely | ||
a security vulnerability. Examples of unwanted actions are polluting globals, | ||
causing an unrecoverable crash, or any other unexpected side effects that can | ||
lead to a loss of confidentiality, integrity, or availability. | ||
|
||
**Node.js trusts everything else**. As some examples this includes: | ||
|
||
1. The developers and infrastructure that runs it. | ||
2. The operating system that Node.js is running under and its configuration, | ||
along with anything under control of the operating system. | ||
3. The code it is asked to run including JavaScript and native code, even if | ||
said code is dynamically loaded, e.g. all dependencies installed from the | ||
npm registry. | ||
The code run inherits all the privileges of the execution user. | ||
4. Inputs provided to it by the code it is asked to run, as it is the | ||
responsibility of the application to perform the required input validations. | ||
5. Any connection used for inspector (debugger protocol) regardless of being | ||
opened by command line options or Node.js APIs, and regardless of the remote | ||
end being on the local machine or remote. | ||
6. The file system when requiring a module. | ||
See <https://nodejs.org/api/modules.html#all-together>. | ||
|
||
Any unexpected behavior from the data manipulation from Node.js Internal | ||
functions are considered a vulnerability. | ||
|
||
In addition to addressing vulnerabilities based on the above, the project works | ||
to avoid APIs and internal implementations that make it "easy" for application | ||
code to use the APIs incorrectly in a way that results in vulnerabilities within | ||
the application code itself. While we don’t consider those vulnerabilities in | ||
Node.js itself and will not necessarily issue a CVE we do want them to be | ||
reported privately to Node.js first. | ||
We often choose to work to improve our APIs based on those reports and issue | ||
fixes either in regular or security releases depending on how much of a risk to | ||
the community they pose. | ||
|
||
### Examples of vulneratibities | ||
|
||
#### Improper Certificate Validation (CWE-295) | ||
|
||
* Node.js provides APIs to validate handling of Subject Alternative Names (SANs) | ||
in certficates used to connect to a TLS/SSL endpoint. If certificates can be | ||
crafted which result in incorrect validation by the Node.js APIs that is | ||
considered a vulnerability. | ||
|
||
#### Inconsistent Interpretation of HTTP Requests (CWE-444) | ||
|
||
* Node.js provides APIs to accept http connections. Those APIs parse the | ||
headers received for a connection and pass them on to the application. | ||
Bugs in parsing those headers which can result in request smuggling are | ||
considered vulnerabilities. | ||
|
||
#### Missing Cryptographic Step (CWE-325) | ||
|
||
* Node.js provides APIs to encrypt data. Bugs that would allow an attacker | ||
to get the orginal data without requiring the encryption key are | ||
considered vulnerabilities. | ||
|
||
#### External Control of System or Configuration Setting (CWE-15) | ||
|
||
* If Node.js automatically loads a configuration file which is not documented | ||
and modification of that configuration can affect the confidentiality of | ||
data protected using the Node.js APIs this is considered a vulnerability. | ||
Comment on lines
+154
to
+155
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is "data protected using the Node.js APIs"? Which of our APIs protect data confidentiality? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When I wrote that a more specific example of what I had in mind was the opessl configuration file, if we don't document that we load the openssl configuration file and from where then by modifying it you can weaken the confidentiality of data communiated over an https connection. Data is also protected through the Crypto APIs that allow data to be encrypted/decrypted. |
||
|
||
### Examples of non-vulneratibities | ||
|
||
#### Malicious Third-Party Modules (CWE-1357) | ||
|
||
* Code is trusted by Node.js, therefore any scenario that requires a malicious | ||
third-party module cannot result in a vulnerability in Node.js. | ||
|
||
#### Prototype Pollution Attacks (CWE-1321) | ||
|
||
* Node.js trusts the inputs provided to it by application code. | ||
It is up to the application to sanitize appropriately, therefore any scenario | ||
that requires control over user input is not considered a vulnerability. | ||
Comment on lines
+164
to
+168
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW, the description is not what I expected based on the header. (There have been some controversial efforts around prototype pollution resistance, but I'm not sure if that's relevant here.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prototype polution often requires some type of tainted external input that is then used in a way that can affect prototype pollution chains or is that a misunderstanding on my part? |
||
|
||
#### Uncontrolled Search Path Element (CWE-427) | ||
|
||
* Node.js trusts the file system in the environment accessible to it. | ||
Therefore, it is not a vulnerability if it accesses/loads files from any path | ||
that is accessible to it. | ||
Comment on lines
+172
to
+174
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure I understand the wording here. Is this, for example, about Node.js following symbolic links that might have been placed on the file system? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, that's more related to the module resolution algorithm itself. For instance, requiring a module that doesn't exist in the current There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and that because we trust the file system that Node.js is running under if we find modules in that upward walk that is not a vulnerability. |
||
|
||
#### External Control of System or Configuration Setting (CWE-15) | ||
|
||
* If Node.js automatically loads a configuration file which is documented | ||
no scenario that requires modification of that configuration file is | ||
considered a vulnerability. | ||
|
||
## Receiving security updates | ||
|
||
Security notifications will be distributed via the following methods. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're assessing if that's a blocker for this PR or it can land without a documentation update for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My current take is that we are documenting/agreeing on what we should do with vulnerabilty reports. As soon as we agree I'm thinking we should take reports, I don't think we load all that many files and if we don't have them documented and people want to help identify them for us, that's not necessarily bad.