Skip to content

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

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.

Sign up for GitHub

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

Merged
KyFaSt merged 1 commit into from
Jan 12, 2026
Merged

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions pages/api/download.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,17 @@ export default function handler(req, res) {
return res.status(400).json({ error: 'Filename is required' });
}

// VULNERABILITY: Path Traversal
// User input is used directly to construct file paths
// An attacker could use input like: "../../../../etc/passwd"
const filePath = path.join(process.cwd(), 'uploads', filename);

// Securely construct a path under the uploads directory
const uploadsRoot = path.join(process.cwd(), 'uploads');
const resolvedPath = path.resolve(uploadsRoot, String(filename));
Copy link

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

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

Converting filename to a string using String(filename) is good for safety, but this should happen before the path resolution. If filename is null or undefined (which shouldn't happen due to the check on line 11, but could theoretically occur), String(null) becomes "null" and String(undefined) becomes "undefined", which would then be used as literal filename strings.

While the current code has a guard on line 11 that checks if (!filename), it's better to be defensive. Consider validating that filename is a non-empty string before proceeding with path operations.

Copilot uses AI. Check for mistakes.

// Ensure the resolved path is within the uploads root to prevent path traversal
if (!resolvedPath.startsWith(uploadsRoot + path.sep) && resolvedPath !== uploadsRoot) {
Copy link

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

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

The path validation allows resolvedPath to equal uploadsRoot (which happens when filename resolves to the uploads directory itself, e.g., when filename is "." or ""). While this doesn't create a security vulnerability, it results in poor error handling: fs.readFileSync will fail when trying to read a directory, causing a 500 error instead of a proper 400 error with "Invalid filename".

The condition should be simplified to only check if the resolved path is a child of the uploads root, not the root itself. Consider changing to: if (!resolvedPath.startsWith(uploadsRoot + path.sep))

Suggested change
if (!resolvedPath.startsWith(uploadsRoot + path.sep) && resolvedPath !== uploadsRoot) {
if (!resolvedPath.startsWith(uploadsRoot + path.sep)) {

Copilot uses AI. Check for mistakes.
return res.status(400).json({ error: 'Invalid filename' });
}
Comment on lines +20 to +22
Copy link

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

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

The current validation doesn't protect against symlink attacks. If a symbolic link exists within the uploads directory that points to a file outside of it, the validation will pass but the file read will access the symlink target outside the allowed directory.

To fully prevent path traversal via symlinks, you should resolve the real path before validation. The PR description mentions this: use fs.realpathSync(resolvedPath) to resolve symlinks, then validate that the real path is still within uploadsRoot. Note that fs.realpathSync will throw an error if the file doesn't exist, so you'll need to handle that case appropriately (either with try-catch or by checking existence first).

See below for a potential fix:

  try {
    // Resolve symlinks and get the real filesystem path
    const realPath = fs.realpathSync(resolvedPath);

    // Ensure the real path is within the uploads root to prevent path traversal and symlink attacks
    if (!realPath.startsWith(uploadsRoot + path.sep) && realPath !== uploadsRoot) {
      return res.status(400).json({ error: 'Invalid filename' });
    }

    const fileContent = fs.readFileSync(realPath, 'utf8');

Copilot uses AI. Check for mistakes.

try {
// Reading file without proper validation
const fileContent = fs.readFileSync(filePath, 'utf8');
const fileContent = fs.readFileSync(resolvedPath, 'utf8');

res.status(200).json({
filename: filename,
Expand Down
Loading