NEW @W-23949583@ [Bug Bounty / H1] Salesforce Graph Engine initialization on Windows

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();

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(

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) => {

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});
+    });
+});

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) => {

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});
+    });
+});