diff --git a/packages/code-analyzer-engine-api/src/utils/java-utils.ts b/packages/code-analyzer-engine-api/src/utils/java-utils.ts index b050d5d9..7b679068 100644 --- a/packages/code-analyzer-engine-api/src/utils/java-utils.ts +++ b/packages/code-analyzer-engine-api/src/utils/java-utils.ts @@ -26,7 +26,13 @@ export class JavaCommandExecutor { this.emitLogEvent(LogLevel.Fine, `Calling command: ${this.javaCommand} ` + allJavaArgs.map(arg => arg.startsWith('-') ? arg : `"${arg}"`).join(' ')); - const javaProcess: ChildProcessWithoutNullStreams = spawn(this.javaCommand, allJavaArgs); + // Pin cwd to this module's trusted install dir (__dirname), not the inherited scanned-repo cwd, so a + // repo-local java.exe can't shadow the real one on Windows, where a bare command name resolves + // cwd-before-PATH (CWE-427). This is the shared executor for the PMD, CPD, and SFGE engines, so this + // single pin protects the actual rule-listing/execution path (not just the version probe) for all three. + // Every java arg (classpaths and I/O files) is absolute, so pinning cwd is behavior-preserving. + // Mirrors the Flow-engine cwd-shadowing fix in PR #495 (W-23791879). + const javaProcess: ChildProcessWithoutNullStreams = spawn(this.javaCommand, allJavaArgs, {cwd: __dirname}); javaProcess.stdout.on('data', (data: Buffer) => { const msg: string = data.toString().trim(); diff --git a/packages/code-analyzer-engine-api/test/utils/utils.test.ts b/packages/code-analyzer-engine-api/test/utils/utils.test.ts index f2670aa8..be24481a 100644 --- a/packages/code-analyzer-engine-api/test/utils/utils.test.ts +++ b/packages/code-analyzer-engine-api/test/utils/utils.test.ts @@ -1,3 +1,8 @@ +import path from "node:path"; +import os from "node:os"; +import fs from "node:fs"; +import cp from "node:child_process"; +import {EventEmitter} from "node:events"; import {FixedClock, indent, JavaCommandExecutor, RealClock} from "../../src/utils"; jest.setTimeout(30_000); @@ -38,6 +43,73 @@ describe('Tests for JavaCommandExecutor', () => { }); }); +function stubJavaExecProcess(exitCode: number = 0): cp.ChildProcessWithoutNullStreams { + // Mimics the surface JavaCommandExecutor.exec() consumes: stdout/stderr streams plus a 'close' event. + const child = Object.assign(new EventEmitter(), {stdout: new EventEmitter(), stderr: new EventEmitter()}); + process.nextTick(() => child.emit('close', exitCode)); + return child as unknown as cp.ChildProcessWithoutNullStreams; +} + +describe('JavaCommandExecutor CWE-427 cwd pinning', () => { + // Under ts-jest the source runs from src/, so java-utils.ts's __dirname (the cwd the spawn is pinned to) is + // the src/utils dir. This is the shared executor for the actual PMD, CPD, and SFGE rule-listing/execution + // path (not just the version probe). + const TRUSTED_DIR: string = path.resolve(__dirname, '..', '..', 'src', 'utils'); + + afterEach(() => jest.restoreAllMocks()); + + // Regression guard: inheriting the scanned-repo cwd would let a repo-local java.exe shadow the real one on + // Windows, where a bare command name resolves cwd-before-PATH. We assert the spawn cwd is pinned to the trusted + // module dir rather than the inherited process cwd. (OS-level shadowing is proven by the Windows integration + // test below; a directly-spawned command cannot be a portable fake-java script across OSes.) + it('pins the java spawn cwd to the trusted engine-api module directory', async () => { + const spawnSpy = jest.spyOn(cp, 'spawn').mockImplementation(() => stubJavaExecProcess(0)); + + await new JavaCommandExecutor('java').exec(['-version']); + + expect(spawnSpy).toHaveBeenCalledWith('java', ['-version'], {cwd: TRUSTED_DIR}); + // The pin must not be the inherited (scanned-repo) cwd — that is the vulnerable behavior. + expect(TRUSTED_DIR).not.toEqual(process.cwd()); + }); + + // End-to-end proof of the security property on the only OS where it is reachable. Windows resolves a bare + // command name cwd-before-PATH, so a repo-local java.exe can shadow the real one; macOS/Linux use PATH only + // (execvp) and never consult cwd, so this test is skipped there. The CI matrix runs the windows-latest leg, + // which provisions a real temurin java on PATH via actions/setup-java. + // + // We plant a genuine java.exe (a copy of a harmless system .exe) in an attacker-controlled dir and chdir there + // to simulate scanning that repo. A real .exe is required: on Node >= 18.20.2/20.12.2 (CI uses Node 20) + // spawning a .bat/.cmd without shell:true throws EINVAL, so a .cmd proxy would never run and the test would + // pass for the wrong reason. Detection is by output: `java --version` prints its banner to stdout and exits 0 + // on JDK 9+, which the planted hostname.exe cannot reproduce. With the {cwd:__dirname} pin the child resolves + // to the real PATH java; remove the pin and the planted exe runs instead, failing both assertions below. + const itOnWindows = process.platform === 'win32' ? it : it.skip; + itOnWindows('does not execute a repo-local java.exe planted in the scanned-repo cwd', async () => { + const attackDir: string = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'cwd-shadow-')); + // A genuine .exe named exactly "java.exe": Windows resolves a bare "java" to it when the cwd is searched. + await fs.promises.copyFile( + path.join(process.env.SystemRoot ?? 'C:\\Windows', 'System32', 'hostname.exe'), + path.join(attackDir, 'java.exe')); + + const originalCwd: string = process.cwd(); + let stdout: string = ''; + let execError: string = ''; + try { + process.chdir(attackDir); // simulate the CLI being invoked from inside the untrusted repo + await new JavaCommandExecutor('java').exec(['--version'], [], line => { stdout += line + '\n'; }); + } catch (err) { + execError = (err as Error).message; + } finally { + process.chdir(originalCwd); + await fs.promises.rm(attackDir, {recursive: true, force: true}); + } + + // The real PATH java must have run (its version banner reached stdout) and not the planted hostname.exe. + expect(stdout.toLowerCase()).toMatch(/java|jdk|openjdk|runtime|hotspot/); + expect(execError).toEqual(''); + }); +}); + describe('Test for indent', () => { it('When using standard indentation then four spaces should be used', () => { expect(indent(`This is a test\nof a multiline\nmessage`)).toEqual( diff --git a/packages/code-analyzer-pmd-engine/src/JavaVersionIdentifier.ts b/packages/code-analyzer-pmd-engine/src/JavaVersionIdentifier.ts index e33c164b..023f44b3 100644 --- a/packages/code-analyzer-pmd-engine/src/JavaVersionIdentifier.ts +++ b/packages/code-analyzer-pmd-engine/src/JavaVersionIdentifier.ts @@ -20,7 +20,11 @@ export class RuntimeJavaVersionIdentifier implements JavaVersionIdentifier { // If instead we used java --version then the output would look something like: // * (from Win10): "openjdk 14 2020-03-17\r\nOpenJDK Runtime Environment (build 14+36-1461)\r\nOpenJDK 64-Bit Server VM (build 14+36-1461, mixed mode, sharing)\r\n" // Notice it doesn't have the word "version" which is why we don't call "--version" but instead call "-version". - const childProcess: cp.ChildProcessWithoutNullStreams = cp.spawn(javaCommand, ['-version']); + // + // Pin cwd to this engine's install dir (__dirname), not the inherited scanned-repo cwd, so a + // repo-local java.exe can't shadow the real one on Windows (CWE-427). Absolute/PATH commands unaffected. + // Shared by the PMD and CPD sub-engines, so this single pin covers both. + const childProcess: cp.ChildProcessWithoutNullStreams = cp.spawn(javaCommand, ['-version'], {cwd: __dirname}); let stderr: string = ''; childProcess.stderr.on('data', (data: Buffer) => { diff --git a/packages/code-analyzer-pmd-engine/test/JavaVersionIdentifier.test.ts b/packages/code-analyzer-pmd-engine/test/JavaVersionIdentifier.test.ts new file mode 100644 index 00000000..cf8be0be --- /dev/null +++ b/packages/code-analyzer-pmd-engine/test/JavaVersionIdentifier.test.ts @@ -0,0 +1,73 @@ +import path from "node:path"; +import cp from "node:child_process"; +import {EventEmitter} from "node:events"; +import {SemVer} from "semver"; +import {_extractJavaVersionFrom, RuntimeJavaVersionIdentifier} from "../src/JavaVersionIdentifier"; + +// Under ts-jest the source runs from src/, so its __dirname (the cwd the spawn is pinned to) is this dir. +const TRUSTED_DIR: string = path.resolve(__dirname, '..', 'src'); + +function stubJavaProcess(stderrLine: string, exitCode: number = 0): cp.ChildProcessWithoutNullStreams { + const child = Object.assign(new EventEmitter(), {stderr: new EventEmitter()}); + process.nextTick(() => { + child.stderr.emit('data', Buffer.from(stderrLine)); + child.emit('exit', exitCode); + }); + return child as unknown as cp.ChildProcessWithoutNullStreams; +} + +describe('Test for _extractJavaVersionFrom helper', () => { + type VERSION_CASE = {description: string, input: string, expected: SemVer}; + const versionCases: VERSION_CASE[] = [ + { + description: 'v11_linux', + input: 'openjdk version "11.0.6" 2020-01-14 LTS\nOpenJDK Runtime Environment Zulu11.37+17-CA (build 11.0.6+10-LTS)\nOpenJDK 64-Bit Server VM Zulu11.37+17-CA (build 11.0.6+10-LTS, mixed mode)\n', + expected: new SemVer('11.0.6') + }, + { + description: 'v8_mac', + input: 'openjdk version "1.8.0_172"\nOpenJDK Runtime Environment (Zulu 8.30.0.2-macosx) (build 1.8.0_172-b01)\nOpenJDK 64-Bit Server VM (Zulu 8.30.0.2-macosx) (build 25.172-b01, mixed mode)\n', + expected: new SemVer('1.8.0') + }, + { + description: 'v12_linux', + input: 'java version "12.0.1" 2019-04-16\nJava(TM) SE Runtime Environment (build 12.0.1+12)\nJava HotSpot(TM) 64-Bit Server VM (build 12.0.1+12, mixed mode, sharing)', + expected: new SemVer('12.0.1') + }, + { // This comes from https://github.com/forcedotcom/sfdx-scanner/issues/1453 + description: 'v17_with_java_options', + input: 'Picked up _JAVA_OPTIONS: -Xmx5g\njava version "17.0.11" 2024-04-16 LTS\nJava(TM) SE Runtime Environment (build 17.0.11+7-LTS-207)', + expected: new SemVer('17.0.11') + }, + { // This type of output typically comes from "java --version" instead of "java -version" but we will try to support it as well + description: 'v14_windows', + input: 'openjdk 14 2020-03-17\r\nOpenJDK Runtime Environment (build 14+36-1461)\r\nOpenJDK 64-Bit Server VM (build 14+36-1461, mixed mode, sharing)\r\n', + expected: new SemVer('14.0.0') + } + ]; + it.each(versionCases)('For version $description, make sure _extractJavaVersionFrom returns expected version', async (caseObj: VERSION_CASE) => { + const version: SemVer = _extractJavaVersionFrom(caseObj.input)!; + expect(version.toString()).toEqual(caseObj.expected.toString()); + }); + + it('Check that _extractJavaVersionFrom returns null if given garbage without version info', async () => { + expect(_extractJavaVersionFrom('this is garbage')).toEqual(null); + }); +}); + +describe('RuntimeJavaVersionIdentifier CWE-427 regression', () => { + afterEach(() => jest.restoreAllMocks()); + + // The java -version spawn must run from a trusted directory; inheriting the scanned-repo cwd would let a + // repo-local java.exe shadow the real one on Windows. We assert the cwd is pinned instead of spawning a + // real fake-java, since a directly-spawned command cannot be a portable script across OSes. + it('pins the java -version spawn cwd to the trusted module directory', async () => { + const spawnSpy = jest.spyOn(cp, 'spawn') + .mockImplementation(() => stubJavaProcess('openjdk version "11.0.6" 2020-01-14')); + + const version = await new RuntimeJavaVersionIdentifier().identifyJavaVersion('java'); + + expect(version?.toString()).toEqual('11.0.6'); + expect(spawnSpy).toHaveBeenCalledWith('java', ['-version'], {cwd: TRUSTED_DIR}); + }); +}); diff --git a/packages/code-analyzer-sfge-engine/src/java-version-identifier.ts b/packages/code-analyzer-sfge-engine/src/java-version-identifier.ts index 86afb981..fc03802e 100644 --- a/packages/code-analyzer-sfge-engine/src/java-version-identifier.ts +++ b/packages/code-analyzer-sfge-engine/src/java-version-identifier.ts @@ -20,7 +20,10 @@ export class RuntimeJavaVersionIdentifier implements JavaVersionIdentifier { // If instead we used java --version then the output would look something like: // * (from Win10): "openjdk 14 2020-03-17\r\nOpenJDK Runtime Environment (build 14+36-1461)\r\nOpenJDK 64-Bit Server VM (build 14+36-1461, mixed mode, sharing)\r\n" // Notice it doesn't have the word "version" which is why we don't call "--version" but instead call "-version". - const childProcess: cp.ChildProcessWithoutNullStreams = cp.spawn(javaCommand, ['-version']); + // + // Pin cwd to this engine's install dir (__dirname), not the inherited scanned-repo cwd, so a + // repo-local java.exe can't shadow the real one on Windows (CWE-427). Absolute/PATH commands unaffected. + const childProcess: cp.ChildProcessWithoutNullStreams = cp.spawn(javaCommand, ['-version'], {cwd: __dirname}); let stderr: string = ''; childProcess.stderr.on('data', (data: Buffer) => { diff --git a/packages/code-analyzer-sfge-engine/test/java-version-identifier.test.ts b/packages/code-analyzer-sfge-engine/test/java-version-identifier.test.ts index 9b074f75..c073f806 100644 --- a/packages/code-analyzer-sfge-engine/test/java-version-identifier.test.ts +++ b/packages/code-analyzer-sfge-engine/test/java-version-identifier.test.ts @@ -1,5 +1,20 @@ +import path from "node:path"; +import cp from "node:child_process"; +import {EventEmitter} from "node:events"; import {SemVer} from "semver"; -import {_extractJavaVersionFrom} from "../src/java-version-identifier"; +import {_extractJavaVersionFrom, RuntimeJavaVersionIdentifier} from "../src/java-version-identifier"; + +// Under ts-jest the source runs from src/, so its __dirname (the cwd the spawn is pinned to) is this dir. +const TRUSTED_DIR: string = path.resolve(__dirname, '..', 'src'); + +function stubJavaProcess(stderrLine: string, exitCode: number = 0): cp.ChildProcessWithoutNullStreams { + const child = Object.assign(new EventEmitter(), {stderr: new EventEmitter()}); + process.nextTick(() => { + child.stderr.emit('data', Buffer.from(stderrLine)); + child.emit('exit', exitCode); + }); + return child as unknown as cp.ChildProcessWithoutNullStreams; +} describe('Test for _extractJavaVersionFrom helper', () => { type VERSION_CASE = {description: string, input: string, expected: SemVer}; @@ -39,3 +54,20 @@ describe('Test for _extractJavaVersionFrom helper', () => { expect(_extractJavaVersionFrom('this is garbage')).toEqual(null); }); }); + +describe('RuntimeJavaVersionIdentifier CWE-427 regression', () => { + afterEach(() => jest.restoreAllMocks()); + + // The java -version spawn must run from a trusted directory; inheriting the scanned-repo cwd would let a + // repo-local java.exe shadow the real one on Windows. We assert the cwd is pinned instead of spawning a + // real fake-java, since a directly-spawned command cannot be a portable script across OSes. + it('pins the java -version spawn cwd to the trusted module directory', async () => { + const spawnSpy = jest.spyOn(cp, 'spawn') + .mockImplementation(() => stubJavaProcess('openjdk version "11.0.6" 2020-01-14')); + + const version = await new RuntimeJavaVersionIdentifier().identifyJavaVersion('java'); + + expect(version?.toString()).toEqual('11.0.6'); + expect(spawnSpy).toHaveBeenCalledWith('java', ['-version'], {cwd: TRUSTED_DIR}); + }); +});