Skip to content

fix: remove shell injection from socket existence check - #239

Merged
GitGab19 merged 1 commit into
stratum-mining:mainfrom
lucasbalieiro:fix-cwe78-on-bitcoin-sock-probe
Aug 10, 2026
Merged

fix: remove shell injection from socket existence check#239
GitGab19 merged 1 commit into
stratum-mining:mainfrom
lucasbalieiro:fix-cwe78-on-bitcoin-sock-probe

Conversation

@lucasbalieiro

Copy link
Copy Markdown
Collaborator

closes https://github.com/project-loupe/audit-sv2-ui/issues/6

needs testing on linux to see if the node.sock probe still working from docker and dev envs

@lucasbalieiro
lucasbalieiro requested a review from GitGab19 August 6, 2026 19:06
Comment on lines +33 to +47
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');
execFileSync(process.execPath, ['-e', bitcoinSocketExistsScript, socketPath], {
stdio: 'ignore',
});
assert.fail('expected non-zero exit for missing socket');
} catch (err) {
assert.ok(
err && typeof err === 'object' && 'status' in err && err.status !== 0,
'missing socket should exit non-zero',
);
}
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clanker suggestion:

This test name says it covers both success and failure, but it only asserts the missing-path failure case. Could we also create the path and assert the script exits successfully? That would catch regressions where the probe always returns non-zero.
Suggested change
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');
execFileSync(process.execPath, ['-e', bitcoinSocketExistsScript, socketPath], {
stdio: 'ignore',
});
assert.fail('expected non-zero exit for missing socket');
} catch (err) {
assert.ok(
err && typeof err === 'object' && 'status' in err && err.status !== 0,
'missing socket should exit non-zero',
);
}
});
test('bitcoinSocketExistsScript exits 0 when the socket path 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 });
}
});

@lucasbalieiro lucasbalieiro Aug 7, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

amended the suggestion

@GitGab19

GitGab19 commented Aug 7, 2026

Copy link
Copy Markdown
Member

Tested on Linux.

  • Dev server (localhost:3001): a listening Unix socket at ~/.sv2-ui-pr239/node.sock validates successfully.
  • Dev server missing-path check: ~/.sv2-ui-pr239-empty/node.sock returns the expected Socket not found error.
  • Dockerized sv2-ui (localhost:8080): the same listening Unix socket validates successfully.

So the node.sock probe still works from both dev and Docker environments on Linux.

@lucasbalieiro
lucasbalieiro force-pushed the fix-cwe78-on-bitcoin-sock-probe branch from 8e031ab to a8de31c Compare August 7, 2026 15:29
@GitGab19
GitGab19 merged commit 313ac5a into stratum-mining:main Aug 10, 2026
6 checks passed
@lucasbalieiro
lucasbalieiro deleted the fix-cwe78-on-bitcoin-sock-probe branch August 10, 2026 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants