fix(security): prevent path traversal in credentials file deletion
Use path.resolve() to normalize paths before comparison in removeIncludeIfCredentials(). The previous startsWith() check was vulnerable to path traversal attacks where a path like "/tmp/runner/../../../etc/passwd" would pass the check but resolve outside RUNNER_TEMP. Also append path.sep to prevent false positives (e.g., /tmp/runner2 matching /tmp/runner).
This commit is contained in:
parent
ca2f66fc96
commit
d9ef76f1ac
2 changed files with 10 additions and 2 deletions
5
dist/index.js
vendored
5
dist/index.js
vendored
|
|
@ -1392,7 +1392,10 @@ class GitConfigHelper {
|
||||||
}
|
}
|
||||||
// Delete only our credentials config file
|
// Delete only our credentials config file
|
||||||
const runnerTemp = process.env['RUNNER_TEMP'];
|
const runnerTemp = process.env['RUNNER_TEMP'];
|
||||||
if (runnerTemp && this.credentialsConfigPath.startsWith(runnerTemp)) {
|
const resolvedCredentialsPath = path.resolve(this.credentialsConfigPath);
|
||||||
|
const resolvedRunnerTemp = runnerTemp ? path.resolve(runnerTemp) : '';
|
||||||
|
if (resolvedRunnerTemp &&
|
||||||
|
resolvedCredentialsPath.startsWith(resolvedRunnerTemp + path.sep)) {
|
||||||
try {
|
try {
|
||||||
yield fs.promises.unlink(this.credentialsConfigPath);
|
yield fs.promises.unlink(this.credentialsConfigPath);
|
||||||
core.info(`Removed credentials config file: ${this.credentialsConfigPath}`);
|
core.info(`Removed credentials config file: ${this.credentialsConfigPath}`);
|
||||||
|
|
|
||||||
|
|
@ -303,7 +303,12 @@ export class GitConfigHelper {
|
||||||
|
|
||||||
// Delete only our credentials config file
|
// Delete only our credentials config file
|
||||||
const runnerTemp = process.env['RUNNER_TEMP']
|
const runnerTemp = process.env['RUNNER_TEMP']
|
||||||
if (runnerTemp && this.credentialsConfigPath.startsWith(runnerTemp)) {
|
const resolvedCredentialsPath = path.resolve(this.credentialsConfigPath)
|
||||||
|
const resolvedRunnerTemp = runnerTemp ? path.resolve(runnerTemp) : ''
|
||||||
|
if (
|
||||||
|
resolvedRunnerTemp &&
|
||||||
|
resolvedCredentialsPath.startsWith(resolvedRunnerTemp + path.sep)
|
||||||
|
) {
|
||||||
try {
|
try {
|
||||||
await fs.promises.unlink(this.credentialsConfigPath)
|
await fs.promises.unlink(this.credentialsConfigPath)
|
||||||
core.info(
|
core.info(
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue