Skip to content

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

Merged
scottsgeorge merged 1 commit intomainfrom
alert-autofix-4
Dec 24, 2025
Merged

Potential fix for code scanning alert no. 4: Uncontrolled data used in path expression#4
scottsgeorge merged 1 commit intomainfrom
alert-autofix-4

Conversation

@scottsgeorge
Copy link
Contributor

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

In general, to fix uncontrolled path usage when serving files, you should: (1) resolve the requested path relative to a known safe root directory, (2) normalize it (removing .. segments), and (3) verify that the resulting absolute path is still within the root directory before using it with filesystem APIs. Optionally, you can further restrict allowed filenames or extensions.

For this code, the best minimal fix without changing visible functionality is: after constructing pathname from rootDir and the requested path, resolve it to an absolute, normalized path (e.g. with path.resolve) and then verify that it is still under rootDir. If it is not, immediately respond with a 403 and do not touch the filesystem. We can also tighten the way we derive safePath from parsedUrl.pathname by making sure we always treat it as a relative path without leading slashes and by forbidding directory traversal segments there as well.

Concretely in scripts/serve.js:

  • Replace the current safePath and pathname logic (lines 28–29) with:
    • create safePath from parsedUrl.pathname || '/' via path.normalize, strip any leading slashes, and if the result starts with '..' or contains path traversal segments, reject it;
    • compute pathname using path.resolve(rootDir, safePath || 'index.html');
    • verify that the resolved pathname starts with rootDir + path.sep or equals rootDir (to prevent /var/www_evil from matching /var/www).
  • Keep the directory handling (lines 31–33), but note that after we resolve and validate pathname, appending 'index.html' within that directory is safe because we stay beneath rootDir.

No new imports are needed; path and fs are already required at the top of the file. The change is confined to the path-handling logic in the request handler.

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 scottsgeorge marked this pull request as ready for review December 24, 2025 16:54
@scottsgeorge scottsgeorge merged commit 872fccc into main Dec 24, 2025
4 checks passed
@chatgpt-codex-connector
Copy link

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

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