Skip to content

Conversation

@dovgopoly
Copy link
Collaborator

@dovgopoly dovgopoly commented Dec 10, 2025

Fixes #8442

parse_filter() expects a JSMN_OBJECT. When not a JSMN_OBJECT is passed to the filter parameter, it called command_fail_badparam() which called command_log() that tried to access c->json_cmd->name, but c->json_cmd was still NULL because find_cmd() hadn't been called yet.

Example that triggered the crash:
./cli/lightning-cli wait -l 'invalid_json_object' -k subsystem=invoices indexname=created nextvalue=0 --network regtest

lightningd: FATAL SIGNAL 11 (version v25.12-21-g3851187-modded)
0x1042d2023 ???
        send_backtrace+0x4f:0
0x1042d20cb ???
        crashdump+0x43:0
0x19fe3b743 ???
        ???:0
0x104180173 command_log
        lightningd/jsonrpc.c:1406
0x10420d8f7 command_fail_badparam
        common/json_command.c:25
0x104181a07 parse_request
        lightningd/jsonrpc.c:1075
0x104181a07 read_json
        lightningd/jsonrpc.c:1216
0x10424c65b next_plan
        ccan/ccan/io/io.c:60
0x10424c65b do_plan
        ccan/ccan/io/io.c:422
0x10424c587 io_ready
        ccan/ccan/io/io.c:439
0x10424dd9b io_loop
        ccan/ccan/io/poll.c:470
0x10417ede7 io_loop_with_timers
        lightningd/io_loop_with_timers.c:22
0x104183a33 main
        lightningd/lightningd.c:1492

@madelinevibes
Copy link
Collaborator

@ShahanaFarooqui has investigated the CI failures... Christian and Rusty are working on resolving some CI problems right now.
Once their fixes are applied, we'll get you to rebase and take a closer look if it still fails.

Shahana also noticed you forgot to add a Changelog entry in the commit.... here is the guide. You'll need to fix this: a very important habit! Not adding a Changelog entry can also fail CI (when it's working as expected).

Well done and congratulations on your first PR for CLN!

@ShahanaFarooqui
Copy link
Collaborator

@dovgopoly Thanks for the PR! I updated your commit message to include a changelog entry. Normally, the CI would fail with a clear error like:

::error::'Changelog' entry is missing in all commits, and 'Changelog-None' not specified in the PR description

However, for some strange reason, the CI didn’t even start the pre-build this time. Adding the changelog message allowed the CI to proceed, and we can investigate further if this issue happens again.

By the way, feel free to edit the commit message as you see fit, I just added it to test the CI flow. :).

@rustyrussell
Copy link
Contributor

Thanks! Great find!

Since this is your first PR I'm going to nitpick!

  1. Please write a quick test (perhaps in tests/test_misc.py) which triggers this: you can have it call out to lightning-cli exactly as you wrote here.
  2. Please include the crash backtrace in the commit msg: this is useful fodder for anyone looking at a similar crash!
  3. Changelog should probably say "Changelog-Fixed: JSON-RPC: fix crash when invalid filter parameter passed in JSON request" since we usually try to start with sub-categories, and CHANGELOG is for users, not (internal) developers.

@dovgopoly
Copy link
Collaborator Author

  1. This segfault is only happened when the --developer mode is off. So it's hard to trigger it in tests. The only approach I found:
def test_filter_with_invalid_json_bug(node_factory):
    l1 = node_factory.get_node(start=False)
    l1.daemon.early_opts = []
    l1.daemon.opts = {k: v for k, v in l1.daemon.opts.items() if not k.startswith('dev')}
    l1.start()

    ret = subprocess.run(['cli/lightning-cli',
                          '--network={}'.format(TEST_NETWORK),
                          '--lightning-dir={}'
                          .format(l1.daemon.lightning_dir),
                          '-l', '1',
                          '-k',
                          'wait',
                          'subsystem=invoices',
                          'indexname=created',
                          'nextvalue=0'])

2-3. Fixed.

Changelog-Fixed: JSON-RPC: fix crash when invalid filter parameter passed in JSON request
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.

bad jsonrpc filter crashes node

4 participants