51369b610e
When an include is not surrounded by empty lines, its content is inlined on the same line as the adjacent content. That can lead to broken tags and other display issues. This PR fixes all such includes and introduces a validation step that forbids introducing the same problem again.
87 lines
2.1 KiB
Plaintext
87 lines
2.1 KiB
Plaintext
== How to fix it in Node.js
|
|
|
|
=== Code examples
|
|
|
|
:code_impact: read
|
|
|
|
include::../../common/fix/code-rationale.adoc[]
|
|
|
|
==== Noncompliant code example
|
|
|
|
[source,javascript,diff-id=1,diff-type=noncompliant]
|
|
----
|
|
const path = require('path');
|
|
|
|
function (req, res) {
|
|
const targetDirectory = "/data/app/resources/"
|
|
const userFilename = path.join(targetDirectory, req.query.filename);
|
|
|
|
let data = fs.readFileSync(userFilename, { encoding: 'utf8', flag: 'r' }); // Noncompliant
|
|
}
|
|
----
|
|
|
|
==== Compliant solution
|
|
|
|
[source,javascript,diff-id=1,diff-type=compliant]
|
|
----
|
|
const path = require('path');
|
|
|
|
function (req, res) {
|
|
const targetDirectory = "/data/app/resources/"
|
|
const userFilename = path.join(targetDirectory, req.query.filename));
|
|
const userFilename = fs.realPath(userFilename);
|
|
|
|
if (!userFilename.startsWith(targetDirectory)) {
|
|
res.status(401).send();
|
|
}
|
|
|
|
let data = fs.readFileSync(userFilename, { encoding: 'utf8', flag: 'r' });
|
|
}
|
|
----
|
|
|
|
=== How does this work?
|
|
|
|
:canonicalization_function: `fs.realPath`
|
|
|
|
include::../../common/fix/self-validation.adoc[]
|
|
|
|
=== Pitfalls
|
|
|
|
include::../../common/pitfalls/partial-path-traversal.adoc[]
|
|
|
|
For example, the following code is vulnerable to partial path injection. Note
|
|
that the string `targetDirectory` does not end with a path separator:
|
|
|
|
|
|
[source, javascript]
|
|
----
|
|
const path = require('path');
|
|
|
|
function (req, res) {
|
|
const targetDirectory = "/data/app/resources"
|
|
const userFilename = path.join(targetDirectory, req.query.filename));
|
|
const userFilename = fs.realPath(userFilename);
|
|
|
|
if (!userFilename.startsWith(targetDirectory)) {
|
|
res.status(401).send();
|
|
}
|
|
|
|
let data = fs.readFileSync(userFilename);
|
|
}
|
|
----
|
|
|
|
This check can be bypassed because `"/Users/Johnny".startsWith("/Users/John")`
|
|
returns `true`. Thus, for validation, `"/Users/John"` should actually be
|
|
`"/Users/John/"`.
|
|
|
|
**Warning**: Some functions remove the terminating path separator in their
|
|
return value. +
|
|
The validation code should be tested to ensure that it cannot be impacted by
|
|
this issue.
|
|
|
|
:joining_docs: https://nodejs.org/api/path.html#pathresolvepaths
|
|
:joining_func: path.resolve
|
|
|
|
include::../../common/pitfalls/path-joining.adoc[]
|
|
|