Conversation
…a used in path expression"
There was a problem hiding this comment.
Pull request overview
This pull request reverts a previous security fix that addressed a path traversal vulnerability in the file download API endpoint. The revert removes secure path validation logic and reintroduces the intentional vulnerability for educational/demonstration purposes in this GitHub Advanced Security demo repository.
Changes:
- Removed secure path validation that prevented path traversal attacks
- Reintroduced direct use of user input in file path construction
- Added explicit vulnerability warning comments
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const filePath = path.join(process.cwd(), 'uploads', filename); | ||
|
|
||
| try { | ||
| const fileContent = fs.readFileSync(resolvedPath, 'utf8'); | ||
| // Reading file without proper validation | ||
| const fileContent = fs.readFileSync(filePath, 'utf8'); |
There was a problem hiding this comment.
This code reintroduces a critical path traversal vulnerability. The filename parameter from user input is directly used in path construction without validation, allowing attackers to access files outside the intended uploads directory using sequences like "../../../etc/passwd". The path.join function does not prevent path traversal attacks. The removed security checks (path.resolve with startsWith validation) were correctly preventing this vulnerability and should be restored.
See below for a potential fix:
// Securely construct and validate the file path to prevent path traversal
const uploadsDir = path.resolve(process.cwd(), 'uploads');
const resolvedPath = path.resolve(uploadsDir, filename);
// Ensure the resolved path is within the uploads directory
if (!resolvedPath.startsWith(uploadsDir + path.sep)) {
return res.status(400).json({ error: 'Invalid filename' });
}
try {
// Reading file after proper validation
const fileContent = fs.readFileSync(resolvedPath, 'utf8');
Reverts #9