Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lightning-cli: fix malformed response bug #7924

Merged

Conversation

Lagrang3
Copy link
Collaborator

Changelog-Fixed: lightning-cli: fix "malformed response" bug

Fixes #7904

There was a wrong assumption that the number of bytes read by cli_read would get us for each correctly
read token two extra CR characters. As a matter of fact one could read enough characters to parse the first
token, but the two extra CR characters are not guaranteed.

@Lagrang3 Lagrang3 force-pushed the lightning-cli-malformed-response branch from 8089be7 to 0ef3fb6 Compare December 10, 2024 09:05
@Lagrang3 Lagrang3 added the bug label Dec 10, 2024
There was a wrong assumption that the number of bytes read
by `cli_read` would get us for each correctly read token
two extra CR characters. As a matter of fact one could read
enough characters to parse the first token, but the two
extra CR characters are not guaranteed.

```
==143570== Memcheck, a memory error detector
==143570== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==143570== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==143570== Command: /home/lagrange/BACKUP/l4-appdata/github/lagrang3/lightning/cli/lightning-cli --lightning-dir=/tmp/askrene_benchmark/lightning -k getroutes source=032ed0d87ba2bd68e3a386717cf2faaae4fa6d6da247986b1997113930e4f841d5 destination=03b2f16bf472dd03c55c2ce9910aab717321db4489cd87df5225adadb08031da4b amount_msat=100000sat final_cltv=6 layers=[] maxfee_msat=500sat
==143570==
==143570== Invalid read of size 1
==143570==    at 0x484A430: memmove (vg_replace_strmem.c:1382)
==143570==    by 0x10C3D2: main (lightning-cli.c:871)
==143570==  Address 0x4a62f80 is 0 bytes after a block of size 1,040 alloc'd
==143570==    at 0x48407B4: malloc (vg_replace_malloc.c:381)
==143570==    by 0x11402E: allocate (tal.c:256)
==143570==    by 0x11471E: tal_alloc_ (tal.c:473)
==143570==    by 0x1147EA: tal_alloc_arr_ (tal.c:517)
==143570==    by 0x10C206: main (lightning-cli.c:816)
==143570==
==143570== Invalid read of size 1
==143570==    at 0x484A43D: memmove (vg_replace_strmem.c:1382)
==143570==    by 0x10C3D2: main (lightning-cli.c:871)
==143570==  Address 0x4a62f81 is 1 bytes after a block of size 1,040 alloc'd
==143570==    at 0x48407B4: malloc (vg_replace_malloc.c:381)
==143570==    by 0x11402E: allocate (tal.c:256)
==143570==    by 0x11471E: tal_alloc_ (tal.c:473)
==143570==    by 0x1147EA: tal_alloc_arr_ (tal.c:517)
==143570==    by 0x10C206: main (lightning-cli.c:816)
==143570==
==143570== Invalid write of size 1
==143570==    at 0x484A433: memmove (vg_replace_strmem.c:1382)
==143570==    by 0x10C3D2: main (lightning-cli.c:871)
==143570==  Address 0x4a62f80 is 0 bytes after a block of size 1,040 alloc'd
==143570==    at 0x48407B4: malloc (vg_replace_malloc.c:381)
==143570==    by 0x11402E: allocate (tal.c:256)
==143570==    by 0x11471E: tal_alloc_ (tal.c:473)
==143570==    by 0x1147EA: tal_alloc_arr_ (tal.c:517)
==143570==    by 0x10C206: main (lightning-cli.c:816)
```

Changelog-Fixed: lightning-cli: fix "malformed response" bug

Signed-off-by: Lagrang3 <[email protected]>
@rustyrussell rustyrussell force-pushed the lightning-cli-malformed-response branch from 0ef3fb6 to 9aeb3f1 Compare December 15, 2024 23:11
@rustyrussell
Copy link
Contributor

Great catch! This is a very old bug, triggered by xpay using more notifications!

I've modified the commit msg slightly to include the valgrind trace, for clarity (GH is disposable, and will vanish one day!)

@rustyrussell rustyrussell added this to the v24.11.1 milestone Dec 15, 2024
@rustyrussell rustyrussell merged commit 7be96ae into ElementsProject:master Dec 16, 2024
36 of 38 checks passed
@Lagrang3 Lagrang3 deleted the lightning-cli-malformed-response branch December 16, 2024 06:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

askrene getroutes produces a malformed response
2 participants