From 4355c8eabdcf2989e1b424abcec137a3d9df4004 Mon Sep 17 00:00:00 2001 From: fusion44 Date: Fri, 3 Jul 2026 19:58:41 +0200 Subject: [PATCH] fix(lightning): prevent shell injection in CLN local calls Two authenticated code paths interpolated user-controlled input into a shell command: - decode_pay_request passed the bolt11 string into _make_local_call, which ran it via create_subprocess_shell; a crafted /lightning/ decode-pay-req request could execute arbitrary commands. Switch _make_local_call to create_subprocess_exec with a discrete argv list. - blitz_cln_unlock interpolated the wallet password into a cl.hsmtool.sh invocation run through a shell, and logged it in the clear. shlex.quote the interpolated values and mark the call sensitive=True. Co-Authored-By: Claude Fable 5 --- app/lightning/impl/cln_grpc.py | 13 ++- .../impl/specializations/blitz_common.py | 7 +- tests/test_cln_shell_safety.py | 93 +++++++++++++++++++ 3 files changed, 107 insertions(+), 6 deletions(-) create mode 100644 tests/test_cln_shell_safety.py diff --git a/app/lightning/impl/cln_grpc.py b/app/lightning/impl/cln_grpc.py index f5afc76..23c96f7 100644 --- a/app/lightning/impl/cln_grpc.py +++ b/app/lightning/impl/cln_grpc.py @@ -40,14 +40,17 @@ from app.lightning.utils import alias_or_empty, generic_grpc_error_handler @logger.catch(exclude=(HTTPException,)) -async def _make_local_call(cmd: str): +async def _make_local_call(*args: str): # FIXME: this is a hack because some of the commands are not exposed # in the CLN grpc interface yet. + # Pass the command as a discrete argv list (create_subprocess_exec, not + # _shell) so user-controlled arguments such as the bolt11 in decodepay + # can never be interpreted as shell syntax. testnet = config("BAPI_NETWORK") == "testnet" - cmd = f"lightning-cli -k {'--testnet ' if testnet else ''}{cmd}" - proc = await asyncio.create_subprocess_shell( - cmd, + argv = ["lightning-cli", "-k", *(["--testnet"] if testnet else []), *args] + proc = await asyncio.create_subprocess_exec( + *argv, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, ) @@ -544,7 +547,7 @@ class LnNodeCLNgRPC(LightningNodeBase): async def decode_pay_request(self, pay_req: str) -> PaymentRequest: logger.trace(f"decode_pay_request(pay_req={pay_req})") - res = await _make_local_call(f"decodepay bolt11={pay_req}") + res = await _make_local_call("decodepay", f"bolt11={pay_req}") if not res: raise HTTPException( diff --git a/app/lightning/impl/specializations/blitz_common.py b/app/lightning/impl/specializations/blitz_common.py index 1b8adda..455e1bf 100644 --- a/app/lightning/impl/specializations/blitz_common.py +++ b/app/lightning/impl/specializations/blitz_common.py @@ -1,4 +1,5 @@ import asyncio +import shlex from fastapi.exceptions import HTTPException from loguru import logger @@ -19,8 +20,12 @@ async def blitz_cln_unlock(network: str, password: str) -> bool: status.HTTP_412_PRECONDITION_FAILED, detail="wallet already unlocked" ) + # shell-quote the interpolated values (password is user-controlled) and + # mark the call sensitive so the plaintext password is never logged res = await exec_bash_command( - f"/home/admin/config.scripts/cl.hsmtool.sh unlock {network} {password}" + "/home/admin/config.scripts/cl.hsmtool.sh unlock " + f"{shlex.quote(network)} {shlex.quote(password)}", + sensitive=True, ) match res: diff --git a/tests/test_cln_shell_safety.py b/tests/test_cln_shell_safety.py new file mode 100644 index 0000000..e4f4972 --- /dev/null +++ b/tests/test_cln_shell_safety.py @@ -0,0 +1,93 @@ +""" +Regression tests: CLN helpers that shell out must not interpret +user-controlled input as shell syntax. + +decode_pay_request forwarded a user-supplied bolt11 string straight into +`asyncio.create_subprocess_shell`, allowing arbitrary command execution +for any authenticated caller. +""" + +import pytest + + +class _FakeProc: + def __init__(self, stdout=b"{}", stderr=b""): + self._stdout = stdout + self._stderr = stderr + + async def communicate(self): + return (self._stdout, self._stderr) + + +@pytest.fixture +def capture_exec(monkeypatch): + """Capture argv passed to create_subprocess_exec and fail loudly if + the shell variant is used at all.""" + from app.lightning.impl import cln_grpc + + calls = {"exec_argv": None, "shell_used": False} + + async def fake_exec(*argv, **kwargs): + calls["exec_argv"] = list(argv) + return _FakeProc() + + async def fake_shell(cmd, **kwargs): + calls["shell_used"] = True + return _FakeProc() + + monkeypatch.setattr(cln_grpc.asyncio, "create_subprocess_exec", fake_exec) + monkeypatch.setattr(cln_grpc.asyncio, "create_subprocess_shell", fake_shell) + monkeypatch.setattr(cln_grpc, "config", lambda key: "mainnet") + return calls + + +async def test_make_local_call_passes_args_without_a_shell(capture_exec): + from app.lightning.impl import cln_grpc + + payload = "lnbc1pdummy; touch /tmp/pwned" + await cln_grpc._make_local_call("decodepay", f"bolt11={payload}") + + assert capture_exec["shell_used"] is False, ( + "must not run user input through a shell" + ) + argv = capture_exec["exec_argv"] + assert argv is not None, "create_subprocess_exec was not called" + # the whole bolt11 value, metacharacters and all, must arrive as one + # discrete argv token so the shell never sees it + assert f"bolt11={payload}" in argv + assert argv[0] == "lightning-cli" + + +async def test_blitz_cln_unlock_quotes_the_password(monkeypatch): + """The CLN unlock password is interpolated into a shell command; it must + be shell-quoted so metacharacters can't inject, and marked sensitive so + it never lands in the logs.""" + import shlex + + from app.api.models import ProcessResult + from app.external.result_type.src.result.result import Ok + from app.lightning.impl.specializations import blitz_common + + captured = {} + + async def fake_exec(command, **kwargs): + captured["command"] = command + captured["kwargs"] = kwargs + # return_code 2 == wrong password -> quick 401, skips the 60s poll + return Ok(ProcessResult(2, "", "")) + + async def fake_redis_get(key): + return "1" # wallet locked + + monkeypatch.setattr(blitz_common, "exec_bash_command", fake_exec) + monkeypatch.setattr(blitz_common, "redis_get", fake_redis_get) + + payload = "pw; touch /tmp/pwned" + with pytest.raises(Exception): + await blitz_common.blitz_cln_unlock("mainnet", payload) + + cmd = captured["command"] + assert shlex.quote(payload) in cmd, "password must be shell-quoted" + assert captured["kwargs"].get("sensitive") is True, ( + "password command must be marked sensitive" + )