From 5c4e35b6c4fcb4286bbee23815f1ebc6a0f627ce Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Thu, 27 Oct 2022 13:32:11 -0300 Subject: [PATCH 1/7] doc: add Node.js Security Best Practices Co-authored-by: Ulises Gascon Co-authored-by: Thomas Gentilhomme Co-authored-by: Facundo Tuesca Co-authored-by: Michael Dawson Co-authored-by: Andrew Hart <829024+arhart@users.noreply.github.com> Co-authored-by: Zbyszek Tenerowicz Co-authored-by: Yagiz Nizipli --- locale/en/docs/guides/index.md | 1 + locale/en/docs/guides/security/index.md | 440 ++++++++++++++++++++++++ 2 files changed, 441 insertions(+) create mode 100644 locale/en/docs/guides/security/index.md diff --git a/locale/en/docs/guides/index.md b/locale/en/docs/guides/index.md index c532e4c299d12..4278573fb559a 100644 --- a/locale/en/docs/guides/index.md +++ b/locale/en/docs/guides/index.md @@ -14,6 +14,7 @@ layout: docs.hbs * [Dockerizing a Node.js web app](/en/docs/guides/nodejs-docker-webapp/) * [Migrating to safe Buffer constructors](/en/docs/guides/buffer-constructor-deprecation/) * [Diagnostics - User Journey](/en/docs/guides/diagnostics/) +* [Security Best Practices](/en/docs/guides/security/) ## Node.js core concepts diff --git a/locale/en/docs/guides/security/index.md b/locale/en/docs/guides/security/index.md new file mode 100644 index 0000000000000..59a72cd75103d --- /dev/null +++ b/locale/en/docs/guides/security/index.md @@ -0,0 +1,440 @@ +--- +title: Node.js Security Best Practices +layout: docs.hbs +--- + +# Node.js Security Best Practices + +## Intent + +This document intends to extend the current [threat model][] and provide extensive +guidelines on how to secure a Node.js application. + +## Document Content + +* Best practices: A simplified condensed way to see the best practices. We can +use [this issue][security guidance issue] or [this guideline][nodejs guideline] +as the starting point. It is important to note that this document is specific +to Node.js, if you are looking for something broad, consider +[OSSF Best Practices][]. +* Attacks explained: illustrate and document in plain English with some code +example (if possible) the attacks that we are mentioning in the threat model. +* Third-Party Libraries: define threats +(typo-squirting attacks, malicious packages...) and best practices regarding +node modules dependencies, etc... + +## Threat List + +### Denial of Service of HTTP server (CWE-400) + +This is an attack where the application becomes unavailable for the purpose it +was designed due to the way it processes incoming HTTP requests. These requests +need not be deliberately crafted by a malicious actor: a misconfigured or buggy +client can also send a pattern of requests to the server that result in a denial +of service. + +HTTP requests are received by the Node.js HTTP server and handed over to the +application code via the registered request handler. The server does not parse +the content of the request body. Therefore any DoS caused by the contents of the +body after they are handed over to the request handler is not a vulnerability in +Node.js itself, since it's the responsibility of the application code to handle +it correctly. + +Ensure that the WebServer handle socket errors properly, for instance, when a +server is created without a error handling, it will be vulnerable to DoS + +```js +const net = require('net'); + +const server = net.createServer(function(socket) { + // socket.on('error', console.error) // this prevents the server to crash + socket.write('Echo server\r\n'); + socket.pipe(socket); +}); + +server.listen(5000, '0.0.0.0'); +``` + +If a _bad request_ is performed the server could crash. + +An example of a DoS attack that is not caused by the request's contents is +[Slowloris][]. In this attack, HTTP requests are sent slowly and fragmented, +one fragment at a time. Until the full request is delivered, the server will +keep resources dedicated to the ongoing request. If enough of these requests +are sent at the same time, the amount of concurrent connections will soon reach +its maximum resulting in a denial of service. This is how the attack depends +not on the request's contents, but on the timing and pattern of the requests +being sent to the server. + +**Mitigations** + +* Use a reverse proxy to receive and forward requests to the Node.js application. +Reverse proxies can provide caching, load balancing, IP blacklisting, etc. which +reduce the probability of a DoS attack being effective. +* Correctly configure the server timeouts, so that connections that are idle or +where requests are arriving too slowly can be dropped. See the different timeouts +in [`http.Server`][], particularly `headersTimeout`, `requestTimeout`, `timeout`, +and `keepAliveTimeout`. +* Limit the number of open sockets per host and in total. See the [http docs][], +particularly `agent.maxSockets`, `agent.maxTotalSockets`, `agent.maxFreeSockets` +and `server.maxRequestsPerSocket`. + +### DNS Rebinding (CWE-346) + +This is an attack that can target Node.js applications being run with the +debugging inspector enabled using the [--inspect switch][]. + +Since websites opened in a web browser can make WebSocket and HTTP requests, +they can target the debugging inspector running locally. +This is usually prevented by the [same-origin policy][] implemented by modern +browsers, which forbids scripts from reaching resources from different origins +(meaning a malicious website cannot read data requested from a local IP address). + +However, through DNS rebinding, an attacker can temporarily control the origin +for their requests so that they seem to originate from a local IP address. +This is done by controlling both a website and the DNS server used to resolve +its IP address, see [DNS Rebinding wiki][] for more details. + +This is not considered a vulnerability in Node.js itself, unless the request has +an invalid origin (such as an invalid IP, or any domain other than localhost) +and Node tries to resolve it via the malicious DNS server. +The expected behavior is that those requests are ignored instead of processed. + +**Mitigations** + +* Disable inspector on _SIGUSR1_ signal by attaching a `process.on(‘SIGUSR1’, …)` +listener to it. +* Do not run the inspector protocol in production. + +### Exposure of Sensitive Information to an Unauthorized Actor (CWE-552) + +All the files and folders included in the current directory are pushed to the +npm registry during the package publication. + +There are some mechanism to control this behavior by defining a blocklist with +`.npmignore` and `.gitignore` or by defining an allowlist in the `package.json` + +**Mitigations** + +* Using `npm publish –dry-run` list all the files to publish. Ensure to review the +content before publishing the package. +* It’s also important to create and maintain ignore files such as `.gitignore` and +`.npmignore`. +Throughout these files, you can specify which files/folders should not be published. +The [files property][] in `package.json` allows the inverse operation +-- allowed list. +* In case of an exposure, make sure to [unpublish the package][]. + +### HTTP Request Smuggling (CWE-444) + +This is an attack that involves two HTTP servers (usually a proxy and a Node.js +application). A client sends an HTTP request that goes first through the +front-end server (the proxy) and then is redirected to the back-end server (the application). +When the front-end and back-end interpret ambiguous HTTP requests differently, +there is potential for an attacker to send a malicious message that won't be +seen by the front-end but will be seen by the back-end, effectively "smuggling" +it past the proxy server. + +See the [CWE-444][] for a more detailed description and examples. + +Due to the fact that this attack depends on Node.js interpreting HTTP requests +differently from an (arbitrary) HTTP server, a successful attack can be due to +a vulnerability in Node.js, the front-end server, or both. +If the way the request is interpreted by Node.js is consistent with the +HTTP specification (see [RFC7230][]), then it is not considered a vulnerability +in Node.js. + +**Mitigations** + +* Do not use the `insecureHTTPParser` option when creating a HTTP Server. +* Configure the front-end server to normalize ambiguous requests. +* Continuously monitor for new HTTP request smuggling vulnerabilities in both +Node.js and the front-end server of choice. +* Use HTTP/2 end to end and disable HTTP downgrading if possible. + +### Information Exposure through Timing Attacks (CWE-208) + +This is an attack type applicable to not Node.js but to all runtimes, and might +lead to information disclosure vulnerabilities. + +A basic authentication method includes email and password as credentials. +User information is retrieved from the input user has supplied from ideally a +DBMS. +Upon retrieving user information, the password is compared within the user +information retrieved from the database. This string comparison takes a longer +time for the same length values. +This comparison, when run for an acceptable amount unwillingly increases the +response time of the request. By comparing the request response times, an +attacker can guess the length and the value of the password in large quantity +of requests. + +**Mitigations** + +* The crypto API exposes a function `timingSafeEqual` to compare actual and +expected sensitive values using a constant-time algorithm. +* For password comparison, you can use the [scrypt][] available also on the +native crypto module. + +### Malicious Third-Party Modules (CWE-1357) + +Currently, in Node.js, any package can access powerful resources such as +network access. +Furthermore, because they also have access to the file system, they can send +any data to anywhere. + +All code running into a node process has the ability to load and run additional +arbitrary code using /p;=(or its equivalents). +All code with file system write access may achieve the same thing by writing to +new or existing files which are loaded. + +**This kind of attack is not considered a vulnerability in Node.js, rather it +is the responsibility of the application to ensure its dependencies are safe +and correct.** + +Node.js has an experimental[¹][experimental-features] +[policy mechanism][] to declare the loaded resource as untrusted or trusted. +However, this policy is not enabled by default. +Be sure to pin dependency versions and run automatic checks for vulnerabilities +using common workflows or npm scripts. +Before installing a package make sure that this package is maintained and +includes all the content you expected in. +Be careful, the Github source code is not always the same as the published one, +validate it in the _node_modules_. + +#### Supply chain attacks + +A supply chain attack on a Node.js application happens when one of its +dependencies (either direct or transitive) is compromised. +This can happen either due to the application being too lax on the specification +of the dependencies (allowing for unwanted updates) and/or common typos in the +specification (vulnerable to [typosquatting][]). + +An attacker that takes control of an upstream package can publish a new version +with malicious code in it. If a Node.js application depends on that package +without being strict on which version is safe to use, the package can be +automatically updated to the latest malicious version, compromising the application. + +Dependencies specified in the `package.json` file can have an exact version number +or a range. However, when pinning a dependency to an exact version, its +transitive dependencies are not themselves pinned. +This still leaves the application vulnerable to unwanted/unexpected updates. + +Possible vector attacks: + +* Typosquatting attacks +* Lockfile poisoning +* Compromised maintainers +* Malicious Packages +* Dependency Confusions + +**Mitigations** + +* Prevent npm from executing arbitrary scripts / Disabling npm postinstall +* Pin dependency versions +* Use lockfiles, which pin every dependency (direct and transitive). + * Use [Mitigations for lockfile poisoning][]. +* Automate checks for new vulnerabilities using CI, with tools like [`npm-audit`][]. + * Tools such as [`Socket`][] can be used to analyze packages with static analysis + to find risky behaviors such as network or filesystem access. +* Use [`npm ci`][] instead of `npm install`. +This enforces the lockfile so that inconsistencies between it and the +_package.json_ file cause an error (instead of silently ignoring the lockfile +in favor of _package.json_). +* Carefully check the _package.json_ file for errors/typos in the names of the +dependencies. + +### Memory Access Violation (CWE-284) + +Memory-based or heap-based attacks depends on a combination of memory management +errors and an exploitable memory allocator. +Like all runtimes, Node.js is vulnerable to these attacks if your projects runs +on a shared machine. +Using a secure heap is useful for preventing sensitive information from leaking +due to pointer overruns and underruns. + +Unfortunately, secure heap is not available on Windows. +More information can be found on Node.js [secure-heap documentation][]. + +**Mitigations** + +* Use `--secure-heap=n` depending on your application where _n_ is the allocated +maximum byte size. +* Do not run your production app in a shared machine. + +### Monkey Patching (CWE-349) + +Monkey patching refers to the modification of properties in runtime aiming to +change the existing behavior. Example: + +```js +// eslint-disable-next-line no-extend-native +Array.prototype.push = function (item) { + // overriding the global [].push +}; +``` + +This is **not** considered a vulnerability, it falls under the following +statement: _“Node.js trusts the code being run”_. + +**Mitigations** + +The `--frozen-intrinsics` flag enables experimental[¹][experimental-features] +frozen intrinsics, which means all the built-in JavaScript objects and functions +are recursively frozen. +Therefore, the following snippet **will not** override the default behavior of +`Array.prototype.push` + +```js +// eslint-disable-next-line no-extend-native +Array.prototype.push = function (item) { + // overriding the global [].push +}; + +// Uncaught: +// TypeError >>: +// Cannot assign to read only property 'push' of object '' +``` + +However, it’s important to mention you can still define new globals and replace +existing globals using `globalThis` +```console +> globalThis.foo = 3; foo; // you can still define new globals +3 +> globalThis.Array = 4; Array; // However, you can also replace existing globals +4 +``` + +Therefore, `Object.freeze(globalThis)` can be used to guarantee no globals will +be replaced. + +### Prototype Pollution Attacks (CWE-1321) + +Prototype pollution refers to the possibility to modify or inject properties +into Javascript language items by abusing the usage of _\_proto\__, +_constructor_, and _prototype_. + +Example: + +```js +const a = {"a": 1, "b": 2}; +const data = JSON.parse('{"__proto__": { "polluted": true}}'); + +const c = Object.assign({}, a, data); +console.log(c.polluted); // true +``` + +This is considered a potential vulnerability inherited from the JavaScript +language. + +**Examples**: + +* [CVE-2022-21824][] (Node.js) +* [CVE-2018-3721][] (3rd Party library: Lodash) + +**Mitigations** + +* Avoid [insecure recursive merges][], see [CVE-2018-16487][]. +* Implement JSON Schema validations for external/untrusted requests. +* Create Objects without prototype by using `Object.create(null)`. +* Freezing the prototype: `Object.freeze(MyObject.prototype)`. +* Disable the `Object.prototype.__proto__` property using `--disable-proto` flag. + +### Uncontrolled Search Path Element (CWE-427) + +Node.js loads modules following the [Module Resolution Algorithm][]. +Therefore, it assumes the directory in which a module is requested +(require) is trusted. + +By that, it means the following application behavior is expected. +Assuming the following directory structure: + +* _app/_ + * _server.js_ + * _auth.js_ + * _auth_ + +If server.js uses `require('./auth')` it will follow the module resolution +algorithm and load _auth_ instead of _auth.js_. + +**Mitigations** + +Using the experimental[¹][experimental-features] +[policy mechanism with integrity checking][] can avoid the above threat. +For the directory described above, one can use the following `policy.json` + +```json +{ + "resources": { + "./app/auth.js": { + "integrity": "sha256-iuGZ6SFVFpMuHUcJciQTIKpIyaQVigMZlvg9Lx66HV8=" + }, + "./app/server.js": { + "dependencies": { + "./auth" : true + }, + "integrity": "sha256-NPtLCQ0ntPPWgfVEgX46ryTNpdvTWdQPoZO3kHo0bKI=" + } + } +} +``` + +Therefore, when requiring the _auth_ module, the system will validate the +integrity and throw an error if doesn’t match the expected one. + +```console +» node --experimental-policy=policy.json app/server.js +node:internal/policy/sri:65 + throw new ERR_SRI_PARSE(str, str[prevIndex], prevIndex); + ^ + +SyntaxError [ERR_SRI_PARSE]: Subresource Integrity string "sha256-iuGZ6SFVFpMuHUcJciQTIKpIyaQVigMZlvg9Lx66HV8=%" had an unexpected "%" at position 51 + at new NodeError (node:internal/errors:393:5) + at Object.parse (node:internal/policy/sri:65:13) + at processEntry (node:internal/policy/manifest:581:38) + at Manifest.assertIntegrity (node:internal/policy/manifest:588:32) + at Module._compile (node:internal/modules/cjs/loader:1119:21) + at Module._extensions..js (node:internal/modules/cjs/loader:1213:10) + at Module.load (node:internal/modules/cjs/loader:1037:32) + at Module._load (node:internal/modules/cjs/loader:878:12) + at Module.require (node:internal/modules/cjs/loader:1061:19) + at require (node:internal/modules/cjs/helpers:99:18) { + code: 'ERR_SRI_PARSE' +} +``` + +Note, it's always recommended the use of `--policy-integrity` to avoid policy mutations. + +## Experimental Features in Production + +The use of experimental features in production isn't recommended. +Experimental features can suffer breaking changes if needed, and their +functionality isn't securely stable. Although, feedback is highly appreciated. + +[threat model]: https://github.com/nodejs/security-wg/issues/799 +[security guidance issue]: https://github.com/nodejs/security-wg/issues/488 +[nodejs guideline]: https://github.com/goldbergyoni/nodebestpractices +[OSSF Best Practices]: https://github.com/ossf/wg-best-practices-os-developers +[Slowloris]: https://en.wikipedia.org/wiki/Slowloris_(computer_security) +[`http.Server`]: https://nodejs.org/api/http.html#class-httpserver +[http docs]: https://nodejs.org/api/http.html +[--inspect switch]: https://nodejs.org/en/docs/guides/debugging-getting-started/ +[same-origin policy]: https://nodejs.org/en/docs/guides/debugging-getting-started/ +[DNS Rebinding wiki]: https://en.wikipedia.org/wiki/DNS_rebinding +[files property]: https://docs.npmjs.com/cli/v8/configuring-npm/package-json#files +[unpublish the package]: https://docs.npmjs.com/unpublishing-packages-from-the-registry +[CWE-444]: https://cwe.mitre.org/data/definitions/444.html +[RFC7230]: https://datatracker.ietf.org/doc/html/rfc7230#section-3 +[policy mechanism]: https://nodejs.org/api/permissions.html#policies +[typosquatting]: https://en.wikipedia.org/wiki/Typosquatting +[Mitigations for lockfile poisoning]: https://securenodejsguidelines.ulisesgascon.com/attacks/lockfile-posioned#mitigation +[`npm ci`]: https://docs.npmjs.com/cli/v8/commands/npm-ci +[secure-heap documentation]: https://nodejs.org/dist/latest-v18.x/docs/api/cli.html#--secure-heapn +[CVE-2022-21824]: https://www.cvedetails.com/cve/CVE-2022-21824/ +[CVE-2018-3721]: https://www.cvedetails.com/cve/CVE-2018-3721/ +[insecure recursive merges]: https://gist.github.com/DaniAkash/b3d7159fddcff0a9ee035bd10e34b277#file-unsafe-merge-js +[CVE-2018-16487]: https://www.cve.org/CVERecord?id=CVE-2018-16487 +[scrypt]: https://nodejs.org/api/crypto.html#cryptoscryptpassword-salt-keylen-options-callback +[Module Resolution Algorithm]: https://nodejs.org/api/modules.html#modules_all_together +[policy mechanism with integrity checking]: https://nodejs.org/api/permissions.html#integrity-checks +[experimental-features]: #experimental-features-in-production +[`Socket`]: https://socket.dev/ From 1f283bd9525c4c10d980c9c46aacd089ebaea4d0 Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Tue, 1 Nov 2022 19:58:28 -0300 Subject: [PATCH 2/7] doc: apply cr suggestions --- locale/en/docs/guides/security/index.md | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/locale/en/docs/guides/security/index.md b/locale/en/docs/guides/security/index.md index 59a72cd75103d..9555d6103b9eb 100644 --- a/locale/en/docs/guides/security/index.md +++ b/locale/en/docs/guides/security/index.md @@ -116,7 +116,7 @@ There are some mechanism to control this behavior by defining a blocklist with **Mitigations** -* Using `npm publish –dry-run` list all the files to publish. Ensure to review the +* Using `npm publish --dry-run` list all the files to publish. Ensure to review the content before publishing the package. * It’s also important to create and maintain ignore files such as `.gitignore` and `.npmignore`. @@ -229,8 +229,10 @@ Possible vector attacks: **Mitigations** -* Prevent npm from executing arbitrary scripts / Disabling npm postinstall -* Pin dependency versions +* Prevent npm from executing arbitrary scripts with `--ignore-scripts` + * Additionally, you can disable it globally with `npm config set ignore-scripts true` +* Pin dependency versions to a specific immutable version, + not a version that is a range or from a mutable source. * Use lockfiles, which pin every dependency (direct and transitive). * Use [Mitigations for lockfile poisoning][]. * Automate checks for new vulnerabilities using CI, with tools like [`npm-audit`][]. @@ -311,7 +313,8 @@ be replaced. Prototype pollution refers to the possibility to modify or inject properties into Javascript language items by abusing the usage of _\_proto\__, -_constructor_, and _prototype_. +_constructor_, _prototype_, and other properties inherited from built-in +prototypes. Example: @@ -321,6 +324,11 @@ const data = JSON.parse('{"__proto__": { "polluted": true}}'); const c = Object.assign({}, a, data); console.log(c.polluted); // true + +// Potential DoS +const data2 = JSON.parse('{"__proto__": null}'); +const d = Object.assign(a, data2); +d.hasOwnProperty('b'); // Uncaught TypeError: d.hasOwnProperty is not a function ``` This is considered a potential vulnerability inherited from the JavaScript @@ -338,6 +346,9 @@ language. * Create Objects without prototype by using `Object.create(null)`. * Freezing the prototype: `Object.freeze(MyObject.prototype)`. * Disable the `Object.prototype.__proto__` property using `--disable-proto` flag. +* Check that the property exists directly on the object, not from the prototype +using `Object.hasOwn(obj, keyFromObj)`. +* Avoid using methods from `Object.prototype`. ### Uncontrolled Search Path Element (CWE-427) @@ -370,7 +381,7 @@ For the directory described above, one can use the following `policy.json` }, "./app/server.js": { "dependencies": { - "./auth" : true + "./auth" : "./app/auth.js" }, "integrity": "sha256-NPtLCQ0ntPPWgfVEgX46ryTNpdvTWdQPoZO3kHo0bKI=" } From 0a3b883e840db2923fd3158b53561cc73b48a69a Mon Sep 17 00:00:00 2001 From: Maledong Date: Fri, 4 Nov 2022 11:27:36 +0800 Subject: [PATCH 3/7] fix the lint error Ignore the js error because we really need it. For more at https://github.com/eslint/eslint-plugin-markdown#skipping-blocks --- locale/en/docs/guides/security/index.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/locale/en/docs/guides/security/index.md b/locale/en/docs/guides/security/index.md index 9555d6103b9eb..d92a414e1d7f6 100644 --- a/locale/en/docs/guides/security/index.md +++ b/locale/en/docs/guides/security/index.md @@ -318,6 +318,8 @@ prototypes. Example: + + ```js const a = {"a": 1, "b": 2}; const data = JSON.parse('{"__proto__": { "polluted": true}}'); From e4ae565416c75c3bd8b77cbbba850cea5b68cc34 Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Fri, 4 Nov 2022 10:21:27 -0300 Subject: [PATCH 4/7] drop \p= mention --- locale/en/docs/guides/security/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/locale/en/docs/guides/security/index.md b/locale/en/docs/guides/security/index.md index d92a414e1d7f6..0ab90d3bcc9e6 100644 --- a/locale/en/docs/guides/security/index.md +++ b/locale/en/docs/guides/security/index.md @@ -183,7 +183,7 @@ Furthermore, because they also have access to the file system, they can send any data to anywhere. All code running into a node process has the ability to load and run additional -arbitrary code using /p;=(or its equivalents). +arbitrary code by using `eval()`(or its equivalents). All code with file system write access may achieve the same thing by writing to new or existing files which are loaded. From 0fcdd5fb962ca80a410932514d38cfda9cc63158 Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Fri, 4 Nov 2022 10:22:53 -0300 Subject: [PATCH 5/7] drop paragraph dns rebinding --- locale/en/docs/guides/security/index.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/locale/en/docs/guides/security/index.md b/locale/en/docs/guides/security/index.md index 0ab90d3bcc9e6..e5d09c03040f0 100644 --- a/locale/en/docs/guides/security/index.md +++ b/locale/en/docs/guides/security/index.md @@ -95,11 +95,6 @@ for their requests so that they seem to originate from a local IP address. This is done by controlling both a website and the DNS server used to resolve its IP address, see [DNS Rebinding wiki][] for more details. -This is not considered a vulnerability in Node.js itself, unless the request has -an invalid origin (such as an invalid IP, or any domain other than localhost) -and Node tries to resolve it via the malicious DNS server. -The expected behavior is that those requests are ignored instead of processed. - **Mitigations** * Disable inspector on _SIGUSR1_ signal by attaching a `process.on(‘SIGUSR1’, …)` From 7a76b2dc222331ef84de7e55ba1d7eb90a28793c Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Fri, 4 Nov 2022 10:33:18 -0300 Subject: [PATCH 6/7] apply cr suggestions --- locale/en/docs/guides/security/index.md | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/locale/en/docs/guides/security/index.md b/locale/en/docs/guides/security/index.md index e5d09c03040f0..561295e8efc02 100644 --- a/locale/en/docs/guides/security/index.md +++ b/locale/en/docs/guides/security/index.md @@ -93,7 +93,7 @@ browsers, which forbids scripts from reaching resources from different origins However, through DNS rebinding, an attacker can temporarily control the origin for their requests so that they seem to originate from a local IP address. This is done by controlling both a website and the DNS server used to resolve -its IP address, see [DNS Rebinding wiki][] for more details. +its IP address. See [DNS Rebinding wiki][] for more details. **Mitigations** @@ -182,10 +182,6 @@ arbitrary code by using `eval()`(or its equivalents). All code with file system write access may achieve the same thing by writing to new or existing files which are loaded. -**This kind of attack is not considered a vulnerability in Node.js, rather it -is the responsibility of the application to ensure its dependencies are safe -and correct.** - Node.js has an experimental[¹][experimental-features] [policy mechanism][] to declare the loaded resource as untrusted or trusted. However, this policy is not enabled by default. @@ -270,9 +266,6 @@ Array.prototype.push = function (item) { }; ``` -This is **not** considered a vulnerability, it falls under the following -statement: _“Node.js trusts the code being run”_. - **Mitigations** The `--frozen-intrinsics` flag enables experimental[¹][experimental-features] @@ -311,8 +304,6 @@ into Javascript language items by abusing the usage of _\_proto\__, _constructor_, _prototype_, and other properties inherited from built-in prototypes. -Example: - ```js @@ -328,7 +319,7 @@ const d = Object.assign(a, data2); d.hasOwnProperty('b'); // Uncaught TypeError: d.hasOwnProperty is not a function ``` -This is considered a potential vulnerability inherited from the JavaScript +This is a potential vulnerability inherited from the JavaScript language. **Examples**: From 0c9bd600311c61b06e6fb57930e4f1e6cec2fc35 Mon Sep 17 00:00:00 2001 From: Rafael Gonzaga Date: Wed, 9 Nov 2022 16:21:55 -0300 Subject: [PATCH 7/7] doc: apply suggestions from code review Co-authored-by: Deian Stefan --- locale/en/docs/guides/security/index.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/locale/en/docs/guides/security/index.md b/locale/en/docs/guides/security/index.md index 561295e8efc02..655e0ad54df11 100644 --- a/locale/en/docs/guides/security/index.md +++ b/locale/en/docs/guides/security/index.md @@ -149,14 +149,14 @@ Node.js and the front-end server of choice. ### Information Exposure through Timing Attacks (CWE-208) -This is an attack type applicable to not Node.js but to all runtimes, and might -lead to information disclosure vulnerabilities. +This is an attack that allows the attacker to learn potentially sensitive information by, for example, measuring how long +it takes for the application to respond to a request. This attack is not specific to Node.js and can target almost all runtimes. -A basic authentication method includes email and password as credentials. +The attack is possible whenever the application uses a secret in a timing-sensitive operation (e.g., branch). Consider handling authentication in typical application. Here, a basic authentication method includes email and password as credentials. User information is retrieved from the input user has supplied from ideally a DBMS. Upon retrieving user information, the password is compared within the user -information retrieved from the database. This string comparison takes a longer +information retrieved from the database. Using the built-in string comparison takes a longer time for the same length values. This comparison, when run for an acceptable amount unwillingly increases the response time of the request. By comparing the request response times, an @@ -170,6 +170,7 @@ expected sensitive values using a constant-time algorithm. * For password comparison, you can use the [scrypt][] available also on the native crypto module. +* More generally, avoid using secrets in variable-time operations. This includes branching on secrets and, when the attacker could be co-located on the same infrastructure (e.g., same cloud machine), using a secret as an index into memory. Writing constant-time code in JavaScript is hard (partly because of the JIT). For crypto applications, use the built-in crypto APIs or WebAssembly (for algorithms not implemented in natively). ### Malicious Third-Party Modules (CWE-1357) Currently, in Node.js, any package can access powerful resources such as