Skip to content

Commit 5d72e21

Browse files
Merge pull request #78 from codacy/escape-characters
fix: Escape special characters
2 parents 7f941f2 + 0a52fc2 commit 5d72e21

File tree

3 files changed

+68
-12
lines changed

3 files changed

+68
-12
lines changed

src/cli/CodacyCli.ts

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
export const CODACY_FOLDER_NAME = '.codacy';
22
import { exec } from 'child_process';
33
import { Log } from 'sarif';
4+
import * as path from 'path';
45

56
// Set a larger buffer size (10MB)
67
const MAX_BUFFER_SIZE = 1024 * 1024 * 10;
@@ -38,8 +39,39 @@ export abstract class CodacyCli {
3839
this._cliCommand = command;
3940
}
4041

42+
protected isPathSafe(filePath: string): boolean {
43+
// Reject null bytes (always a security risk)
44+
if (filePath.includes('\0')) {
45+
return false;
46+
}
47+
48+
// Reject all control characters (including newline, tab, carriage return)
49+
// as they are very unusual for file names
50+
// eslint-disable-next-line no-control-regex -- Intentionally checking for control chars to reject them for security
51+
const hasUnsafeControlChars = /[\x00-\x1F\x7F]/.test(filePath);
52+
if (hasUnsafeControlChars) {
53+
return false;
54+
}
55+
56+
// Resolve the path to check for path traversal attempts
57+
const resolvedPath = path.resolve(this.rootPath, filePath);
58+
const normalizedRoot = path.normalize(this.rootPath);
59+
// Check if the resolved path is within the workspace
60+
if (!resolvedPath.startsWith(normalizedRoot)) {
61+
return false;
62+
}
63+
64+
return true;
65+
}
66+
4167
protected preparePathForExec(path: string): string {
42-
return path;
68+
// Validate path security before escaping
69+
if (!this.isPathSafe(path)) {
70+
throw new Error(`Unsafe file path rejected: ${path}`);
71+
}
72+
73+
// Escape special characters for shell execution
74+
return path.replace(/([\s'"\\;&|`$()[\]{}*?~<>])/g, '\\$1');
4375
}
4476

4577
protected execAsync(
@@ -48,11 +80,20 @@ export abstract class CodacyCli {
4880
): Promise<{ stdout: string; stderr: string }> {
4981
// stringyfy the args
5082
const argsString = Object.entries(args || {})
51-
.map(([key, value]) => `--${key} ${value}`)
83+
.map(([key, value]) => {
84+
// Validate argument key (should be alphanumeric and hyphens only)
85+
if (!/^[a-zA-Z0-9-]+$/.test(key)) {
86+
throw new Error(`Invalid argument key: ${key}`);
87+
}
88+
89+
// Escape the value to prevent injection
90+
const escapedValue = value.replace(/([\s'"\\;&|`$()[\]{}*?~<>])/g, '\\$1');
91+
return `--${key} ${escapedValue}`;
92+
})
5293
.join(' ');
5394

54-
// Add the args to the command and remove any shell metacharacters
55-
const cmd = `${command} ${argsString}`.trim().replace(/[;&|`$]/g, '');
95+
// Build the command - no need to strip characters since we've already escaped them properly
96+
const cmd = `${command} ${argsString}`.trim();
5697

5798
return new Promise((resolve, reject) => {
5899
exec(

src/cli/WinWSLCodacyCli.ts

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,29 +9,44 @@ export class WinWSLCodacyCli extends MacCodacyCli {
99
}
1010

1111
private static toWSLPath(path: string): string {
12-
// Convert Windows path to WSL path
13-
// Example: C:\Users\user\project -> /mnt/c/Users/user/project
14-
const wslPath = path.replace(/\\/g, '/').replace(/^([a-zA-Z]):/, '/mnt/$1');
12+
// First, remove outer quotes if present
13+
const cleanPath = path.replace(/^["']|["']$/g, '');
14+
// Convert backslashes to slashes and handle drive letter
15+
const wslPath = cleanPath
16+
.replace(/\\/g, '/')
17+
.replace(/^([a-zA-Z]):/, (match, letter) => `/mnt/${letter.toLowerCase()}`);
1518
return wslPath;
1619
}
1720

1821
private static fromWSLPath(path: string): string {
1922
// Convert WSL path to Windows path
2023
// Example: /mnt/c/Users/user/project -> C:\Users\user\project
21-
const windowsPath = path.replace(/^\/mnt\/([a-zA-Z])/, '$1:').replace(/\//g, '\\');
24+
const windowsPath = path
25+
.replace(/^'\/mnt\/([a-zA-Z])/, (match, letter) => `'${letter.toUpperCase()}:`)
26+
.replace(/^\/mnt\/([a-zA-Z])/, (match, letter) => `${letter.toUpperCase()}:`)
27+
.replace(/\//g, '\\');
2228
return windowsPath;
2329
}
2430

2531
protected preparePathForExec(path: string): string {
26-
// Convert the path to WSL format
27-
return WinWSLCodacyCli.toWSLPath(path);
32+
// Convert WSL path to Windows format for validation
33+
const winFilePath = path.startsWith('/mnt/') ? WinWSLCodacyCli.fromWSLPath(path) : path;
34+
35+
// Validate path security before escaping
36+
if (!this.isPathSafe(winFilePath)) {
37+
throw new Error(`Unsafe file path rejected: ${winFilePath}`);
38+
}
39+
// Convert to WSL format and escape special characters
40+
const wslPath = WinWSLCodacyCli.toWSLPath(winFilePath);
41+
const escapedPath = wslPath.replace(/([\s'"\\;&|`$()[\]{}*?~<>])/g, '\\$1');
42+
return `'${escapedPath}'`;
2843
}
2944

3045
protected async execAsync(
3146
command: string,
3247
args?: Record<string, string>
3348
): Promise<{ stdout: string; stderr: string }> {
34-
return await super.execAsync(`wsl ${command}`, args);
49+
return await super.execAsync(`wsl bash -c "${command}"`, args);
3550
}
3651

3752
protected getCliCommand(): string {

src/handlers/cliAnalyze.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ export const cliAnalyzeHandler = async (args: any) => {
55

66
try {
77
const results = await cli.analyze({
8-
file: args.file,
8+
file: `'${args.file}'`,
99
tool: args.tool,
1010
});
1111

0 commit comments

Comments
 (0)