Skip to content
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

fix(connector-fabric): uncontrolled data used in path expression #1909

Closed
petermetz opened this issue Mar 14, 2022 · 0 comments · Fixed by #1910
Closed

fix(connector-fabric): uncontrolled data used in path expression #1909

petermetz opened this issue Mar 14, 2022 · 0 comments · Fixed by #1910
Assignees
Labels
dependencies Pull requests that update a dependency file Fabric Security Related to existing or potential security vulnerabilities

Comments

@petermetz
Copy link
Contributor

https://github.com/hyperledger/cactus/security/code-scanning/40

Uncontrolled data used in path expression

Tool
CodeQL
Rule ID
js/path-injection
Query
View source

Accessing files using paths constructed from user-controlled data can allow an attacker to access unexpected resources. This can result in sensitive information being revealed or deleted, or an attacker being able to influence behavior by modifying unexpected files.
Recommendation

Validate user input before using it to construct a file path, either using an off-the-shelf library like the sanitize-filename npm package, or by performing custom validation.

Ideally, follow these rules:

Do not allow more than a single "." character.
Do not allow directory separators such as "/" or "\" (depending on the file system).
Do not rely on simply replacing problematic sequences such as "../". For example, after applying this filter to ".../...//", the resulting string would still be "../".
Use a whitelist of known good patterns.

Example

In the first example, a file name is read from an HTTP request and then used to access a file. However, a malicious user could enter a file name which is an absolute path, such as "/etc/passwd".

In the second example, it appears that the user is restricted to opening a file within the "user" home directory. However, a malicious user could enter a file name containing special characters. For example, the string "../../etc/passwd" will result in the code reading the file located at "/home/user/../../etc/passwd", which is the system's password file. This file would then be sent back to the user, giving them access to all the system's passwords.

var fs = require('fs'),
    http = require('http'),
    url = require('url');

var server = http.createServer(function(req, res) {
  let path = url.parse(req.url, true).query.path;

  // BAD: This could read any file on the file system
  res.write(fs.readFileSync(path));

  // BAD: This could still read any file on the file system
  res.write(fs.readFileSync("/home/user/" + path));
});

References

OWASP: Path Traversal.
npm: sanitize-filename package.
Common Weakness Enumeration: CWE-22.
Common Weakness Enumeration: CWE-23.
Common Weakness Enumeration: CWE-36.
Common Weakness Enumeration: CWE-73.
Common Weakness Enumeration: CWE-99.
@petermetz petermetz added dependencies Pull requests that update a dependency file Fabric Security Related to existing or potential security vulnerabilities labels Mar 14, 2022
@petermetz petermetz self-assigned this Mar 14, 2022
petermetz added a commit to petermetz/cacti that referenced this issue Mar 14, 2022
Starts using the `sanitize-filename` npm package to
secure the Fabric ledger connector against malicious
user input when it comes to file paths of the golang
source codes that can be deployed through it.

https://www.npmjs.com/package/sanitize-filename

Fixes hyperledger-cacti#1909

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
petermetz added a commit that referenced this issue Mar 14, 2022
Starts using the `sanitize-filename` npm package to
secure the Fabric ledger connector against malicious
user input when it comes to file paths of the golang
source codes that can be deployed through it.

https://www.npmjs.com/package/sanitize-filename

Fixes #1909

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file Fabric Security Related to existing or potential security vulnerabilities
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant