Skip to content

Potential fix for code scanning alert no. 5: Uncontrolled data used in path expression - #3

Closed
scottsgeorge wants to merge 1 commit into
mainfrom
alert-autofix-5
Closed

scottsgeorge wants to merge 1 commit into
mainfrom
alert-autofix-5

Conversation

@scottsgeorge

Copy link
Copy Markdown
Contributor

Potential fix for https://github.com/hellopaywaz/paywaz-docs/security/code-scanning/5

In general terms, the fix is to enforce that any path derived from req.url is constrained to a specific root directory. This is usually done by resolving the request path against the root directory, normalizing it (removing .. segments), and then verifying that the resulting absolute path still starts with the intended root directory path. If it does not, the server should reject the request (for example, with HTTP 403) instead of attempting to read the file.

The best change here, without altering existing behavior beyond security, is: after obtaining safePath, compute pathname with path.resolve(rootDir, safePath) rather than path.join, then check that pathname starts with rootDir + path.sep (or equals rootDir), before using it. If the check fails, respond with 403 and return. This ensures that even if safePath contains traversal segments or tricks that bypass the current regex, the resolved path cannot escape rootDir. The rest of the logic (adding index.html for directories, determining MIME types, etc.) can remain unchanged, but must operate on the validated pathname. Concretely, in scripts/serve.js around lines 28–35, replace the safePath computation and path.join(rootDir, safePath) usage with a path.resolve call and a containment check; if invalid, send a 403 response and skip fs.readFile.

No new external libraries are required: Node's built-in path and fs modules already provide what we need. We will keep the imports as they are and only adjust the calculation of safePath/pathname and add a small validation block.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…n path expression

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@scottsgeorge

Copy link
Copy Markdown
Contributor Author

Closing this old Copilot Autofix draft because PR #4 was already merged and appears to address the same uncontrolled path handling issue in scripts/serve.js with a more complete containment check.

If code scanning alert #5 is still open after the merged fix, we should open a fresh PR from current main rather than merge this stale, non-mergeable draft.

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.

1 participant