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

Improve path validity verification #8

Open
4 tasks
Xkonti opened this issue Aug 25, 2023 · 0 comments
Open
4 tasks

Improve path validity verification #8

Xkonti opened this issue Aug 25, 2023 · 0 comments
Labels

Comments

@Xkonti
Copy link
Owner

Xkonti commented Aug 25, 2023

TODO:

  • Normalize paths written into the config - this way we can avoid situations where later normalizations don't match values form config
  • Sanitize paths written into the config - if after sanitization, the config value has changed, it should stop the application completely - we don't want to cause the application to misbehave during runtime
  • Check if paths end up within the data folder using normalization in isPathValid function
  • Check if paths stay the same after sanitization - fail the isPathValid function if not

Explanation

Pretty much all API endpoints deal with a client sending a path to file or a directory. This path needs to be properly verified so that it's not trying to access areas outside of the designated data folder (prevent Directory Traversal Attacks). Currently it's just checking if there are no .. anywhere in the path as it could cause getting from data directory to the root, which shouldn't be possible.

A proper path verification needs to be implemented and replace the isPathValid function inside /src/utils/pathUtils.ts.

Normalization

First step of the validation should be the path normalization. After normalizing the full path it should be easy to verify that the path is still leading to content within the data directory. Example:

const directoryPath = '/data'; // This should be normalized as well when loading data into the config
const clientFilePath = './hello/../world/../documents/notes.md';

// Join both paths to get the absolute path
// In this example it should yield: /data/./hello/../world/../documents/notes.md
const fullPath = path.join(directoryPath, clientFilePath);

// Normalize path to get the end-result path
// In this example it should yield: /data/documents/notes.md
const normalizedPath = path.normalize(fullPath);

// Check if the normalized path is still within the main data directory
if (!normalizedPath.startsWith(directoryPath)) {
  // Error - somebody tried directory traversal attack!
  return false;
}

Path sanitization

Somebody could try some shenanigans with weird characters in file names. To avoid this problem the path should be sanitized before being used. If the path is different after sanitization, that means that something was off. Instead of proceeding, the verification should be failed. Even if the user didn't intend anything wrong at least we'll prevent any unexpected results. The sanitize-filename npm package could be used.

@Xkonti Xkonti added the good first issue Good for newcomers label Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant