-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[Security] Fix MEDIUM vulnerability: javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal #8149
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
base: main
Are you sure you want to change the base?
Conversation
…-traversal.path-join-resolve-traversal.path-join-resolve-traversal Automatically generated security fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR attempts to fix a path traversal vulnerability in the build metadata script by introducing input sanitization before path operations. However, the fix has several significant issues that need to be addressed.
Key Issues Identified:
- Incomplete sanitization: Unsanitized variables are still used in downstream operations (lines 58, 66), potentially allowing path traversal sequences to persist in the output
- Suboptimal approach: The regex-based sanitization can be bypassed and is less robust than using
path.basename() - Functionality risk: The sanitization may break legitimate functionality since it constructs paths that don't match actual filesystem locations
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const sanitizedCategory = sanitizeName(category); | ||
| const categoryPath = path.resolve(rootPath, sanitizedCategory); | ||
| const examplesFiles = getDirFiles(categoryPath); | ||
| const categoryKebab = toKebabCase(category); |
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sanitization is applied to category, but line 58 still uses the original unsanitized category variable to generate categoryKebab. This inconsistency means that if category contained path traversal sequences, they would be preserved in the kebab case conversion and potentially stored in the metadata output.
For consistency and security, use the sanitized version:
const categoryKebab = toKebabCase(sanitizedCategory);| const categoryKebab = toKebabCase(category); | |
| const categoryKebab = toKebabCase(sanitizedCategory); |
| const examplePath = path.resolve(`${categoryPath}/${exampleFile}`); | ||
| const sanitizedExampleFile = sanitizeName(exampleFile); | ||
| const examplePath = path.resolve(categoryPath, sanitizedExampleFile); | ||
| const exampleName = exampleFile.split('.').shift() ?? ''; |
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sanitization is applied to exampleFile, but line 66 still uses the original unsanitized exampleFile variable to extract the example name. This inconsistency means that if exampleFile contained path traversal sequences, they would be preserved in exampleName and subsequently in exampleNameKebab at line 67, potentially storing malicious paths in the metadata output.
For consistency and security, use the sanitized version:
const exampleName = sanitizedExampleFile.split('.').shift() ?? '';| const exampleName = exampleFile.split('.').shift() ?? ''; | |
| const exampleName = sanitizedExampleFile.split('.').shift() ?? ''; |
kpal81xd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of stripping invalid characters i just restricted the characters only include lowercase and underscores
|
|
||
| const main = () => { | ||
| const rootPath = 'src/examples'; | ||
| const categories = getDirFiles(rootPath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const categories = getDirFiles(rootPath); | |
| const categories = getDirFiles(rootPath).reduce((acc, file) => { | |
| if (!/^[a-z-]+$/.test(file)) { | |
| console.warn(`skipping invalid category folder: ${file}`); | |
| return acc; | |
| } | |
| acc.push(file); | |
| return acc; | |
| }, /** @type {string[]} */ ([])); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we want to use this for example names to I would just move this reduce up to getDirFiles and call it getSantizedDirFiles
|
|
||
| const main = () => { | ||
| const rootPath = 'src/examples'; | ||
| const categories = getDirFiles(rootPath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we want to use this for example names to I would just move this reduce up to getDirFiles and call it getSantizedDirFiles
Security Fix
This PR addresses a MEDIUM severity vulnerability detected by our security scanner.
Security Impact Assessment
Evidence: Proof-of-Concept Exploitation Demo
This demonstration shows how the vulnerability could be exploited to help you understand its severity and prioritize remediation.
How This Vulnerability Can Be Exploited
The vulnerability in
examples/scripts/build-metadata.mjsarises from unsanitized user input being passed topath.joinorpath.resolve, allowing an attacker to perform path traversal attacks. In the context of the PlayCanvas engine repository, this script is likely used during development or build processes for examples, where it constructs file paths based on command-line arguments or configuration inputs. An attacker with the ability to influence these inputs (e.g., via a compromised build environment, malicious pull request, or local execution on a developer's machine) could traverse the filesystem to access arbitrary files outside the intended directory.The vulnerability in
examples/scripts/build-metadata.mjsarises from unsanitized user input being passed topath.joinorpath.resolve, allowing an attacker to perform path traversal attacks. In the context of the PlayCanvas engine repository, this script is likely used during development or build processes for examples, where it constructs file paths based on command-line arguments or configuration inputs. An attacker with the ability to influence these inputs (e.g., via a compromised build environment, malicious pull request, or local execution on a developer's machine) could traverse the filesystem to access arbitrary files outside the intended directory.Exploitation Impact Assessment
Vulnerability Details
javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversalexamples/scripts/build-metadata.mjspath.joinorpath.resolvefunction. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.Changes Made
This automated fix addresses the vulnerability by applying security best practices.
Files Modified
examples/scripts/build-metadata.mjsVerification
This fix has been automatically verified through:
🤖 This PR was automatically generated.