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

Fixed-constraints-added-param-passed #124

Conversation

ShivamAher30
Copy link
Contributor

@ShivamAher30 ShivamAher30 commented Jun 9, 2024

#120 Issue: Path Traversal Vulnerability via Illegal URL Destination Parameter

Problem:
Normally, when passing the destination parameter, a file is downloaded. However, if characters like ../ are added and passed as a parameter after URL encoding, it enables access to files outside of the directory. For instance, the string ../test/jest-e2e.json, when URL encoded, can be passed as the destination parameter: localhost:3000/files/download/..%2Ftest%2Fjest-e2e.json. This results in the file being served as a response, potentially exposing sensitive information.
How to Recreate:

  1. Pass a URL-encoded string containing ../ as the destination parameter.
  2. Access the corresponding endpoint, for example: localhost:3000/files/download/..%2Ftest%2Fjest-e2e.json.
  3. The content of the jest-e2e.json file is served as a response, revealing potentially sensitive data.
    How to Recreate:
  4. Pass a URL-encoded string containing ../ as the destination parameter.
  5. Access the corresponding endpoint, for example: localhost:3000/files/download/..%2Ftest%2Fjest-e2e.json.TestCase-1
  6. The content of the jest-e2e.json file is served as a response, revealing potentially sensitive data.
  7. Also passing ../.env as url encoded %2E%2E%2F%2Eenv leads to the path traversal and .env file being served TestCase-2

Solution:
To mitigate this vulnerability, we need to validate the destination parameter to ensure it does not contain any illegal characters or patterns that could lead to path traversal attacks. We can achieve this by implementing server-side validation and rejecting requests with invalid destination parameters.

Changes:

  1. Implement validation checks for the destination parameter in the affected endpoint handler controller.
  2. If the destination parameter contains illegal characters or patterns indicative of path traversal attempts, return an appropriate error response and do not serve the requested file.

Copy link
Member

@techsavvyash techsavvyash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add test cases

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we have this file?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we have this file?

@@ -54,6 +54,10 @@ export class FileUploadController {
@Res() res: FastifyReply,
): Promise<void> {
try {
const pathTraversalRegex = /\.\.\//;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will it handle all cases? i think the regex should just be allowing the <A-Z|0-8>.<A-Z> with the extension being optional. Please add relevant test cases to show that it works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants