From a8de31c91ee8dfd3a3cde00e0170833b29c332ee Mon Sep 17 00:00:00 2001 From: Lucas Balieiro <37416577+lucasbalieiro@users.noreply.github.com> Date: Thu, 6 Aug 2026 16:02:24 -0300 Subject: [PATCH] fix: remove shell injection from socket existence check --- server/src/bitcoin-socket-exists.test.ts | 52 ++++++++++++++++++++++++ server/src/bitcoin-socket-exists.ts | 7 +++- 2 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 server/src/bitcoin-socket-exists.test.ts diff --git a/server/src/bitcoin-socket-exists.test.ts b/server/src/bitcoin-socket-exists.test.ts new file mode 100644 index 00000000..30ec0a11 --- /dev/null +++ b/server/src/bitcoin-socket-exists.test.ts @@ -0,0 +1,52 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { execFileSync } from 'node:child_process'; +import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { bitcoinSocketExistsScript } from './bitcoin-socket-exists.js'; + +test('bitcoinSocketExistsScript must not execute shell commands embedded in the socket path', () => { + const tmp = mkdtempSync(path.join(tmpdir(), 'sock-inj-')); + const marker = path.join(tmp, 'pwned'); + + const maliciousPath = '/tmp/$(touch ' + marker + ')/node.sock'; + + try { + execFileSync(process.execPath, ['-e', bitcoinSocketExistsScript, maliciousPath], { + stdio: 'ignore', + }); + } catch { + // A non-zero exit (socket absent) is expected and irrelevant here. + } + + const injected = existsSync(marker); + rmSync(tmp, { recursive: true, force: true }); + + assert.equal( + injected, + false, + 'socket path was interpreted as a shell command (command injection via execSync)', + ); +}); + +test('bitcoinSocketExistsScript exits 0 when the socket exists and non-zero otherwise', () => { + const tmp = mkdtempSync(path.join(tmpdir(), 'sock-exists-')); + try { + const socketPath = path.join(tmp, 'node.sock'); + assert.throws( + () => + execFileSync(process.execPath, ['-e', bitcoinSocketExistsScript, socketPath], { + stdio: 'ignore', + }), + /Command failed/, + 'missing socket path should exit non-zero', + ); + writeFileSync(socketPath, ''); + execFileSync(process.execPath, ['-e', bitcoinSocketExistsScript, socketPath], { + stdio: 'ignore', + }); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } +}); diff --git a/server/src/bitcoin-socket-exists.ts b/server/src/bitcoin-socket-exists.ts index f29764ff..a1215940 100644 --- a/server/src/bitcoin-socket-exists.ts +++ b/server/src/bitcoin-socket-exists.ts @@ -1,12 +1,15 @@ export const bitcoinSocketExistsScript = `const fs = require('fs'); -const { execSync } = require('child_process'); const socketPath = process.argv[1]; const dir = require('path').dirname(socketPath); try { console.log('DEBUG: Checking path:', socketPath); - console.log('DEBUG: Directory contents:', execSync('ls -la ' + dir).toString()); + // DO NOT REMOVE: sadly, on Docker Desktop for macOS, a host Unix socket is not + // visible to fs.existsSync until its parent directory has been read. + // This readdirSync populates the file-sharing cache that the existence + // check below relies on. + fs.readdirSync(dir); console.log('DEBUG: exists:', fs.existsSync(socketPath)); } catch(e) { console.log('DEBUG ERROR:', e.message);