mirror of
https://github.com/fusion44/blitz_api.git
synced 2026-08-13 11:52:45 +02:00
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 <noreply@anthropic.com>
This commit is contained in:
parent
ba78b10aa2
commit
4355c8eabd
3 changed files with 107 additions and 6 deletions
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
93
tests/test_cln_shell_safety.py
Normal file
93
tests/test_cln_shell_safety.py
Normal file
|
|
@ -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"
|
||||
)
|
||||
Loading…
Add table
Add a link
Reference in a new issue