askrene: fix crash loading node bias with description
What the developer wrote
Authored by Vincenzo Palazzo
askrene: fix crash loading node bias with description
load_node_bias() passed take(description) to two consecutive
set_node_bias() calls. The first call's tal_strdup() consumes the
take (tal_resize_ + tal_steal), so the second take() was on freed
memory and we aborted in to_tal_hdr() with "Not a valid header"
while loading the layer at startup. Since askrene is an important
plugin, lightningd shuts down and the node cannot restart at all.
The description is already a copy off tmpctx, so simply don't take()
it: set_node_bias() strdups it into the bias anyway.
With this, the test from the previous commit passes.
Fixes: #9433
Reported-by: endothermicdev
Changelog-Fixed: askrene: node failed to start (`exited before replying to init`) when a persistent layer contains a node bias with a description
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
What changed, and why it matters
This commit fixes a bug in Core Lightning's askrene plugin that could prevent a node from restarting. When a saved routing layer contained a node bias with a description, the plugin accidentally freed the description's memory while using it a second time, causing an immediate crash during startup. The fix removes the incorrect 'take ownership' call so the description remains valid for both uses. There is no evidence this crash can be triggered remotely or used as an attack; it appears to be a reliability bug that only affects nodes that already have such saved data.
Apply the patch. It is a minimal, correct fix for a startup-crash bug. No additional hardening is suggested by the diff. Operators who cannot restart due to this bug can upgrade or temporarily remove the affected persistent askrene layer data after backing it up.
Security signals we found
Use-after-free / double-take of a tal-allocated string during plugin startup
Denial-of-service-like symptom: lightningd aborts before replying to init, node cannot restart
Fixes publicly reported issue #9433 by endothermicdev
Changelog explicitly frames it as a fixed startup failure
Evidence from the diff
In plugins/askrene/layer.c, load_node_bias() called set_node_bias() twice, each with take(description). Because set_node_bias() internally tal_strdup()s the description, the first take() frees/moves the original string, making the second take() operate on freed memory. This produced a tal allocator abort (‘Not a valid header’) during layer load at startup, preventing lightningd from initializing when a persistent layer had a node bias with a description. The fix passes description without take() to both calls, since it is already a copy off tmpctx and set_node_bias() will strdup it into the bias anyway. The accompanying test removes the @pytest.mark.xfail marker, confirming the previously-failing persistence test now passes.
Changed components
plugins/askrene/layer.cload_node_bias()set_node_bias()askrene plugin startup / persistent layer loadingInspect captured patch +2 / −3
### plugins/askrene/layer.c
@@ -773,10 +773,10 @@ static void load_node_bias(struct plugin *plugin,
&in_bias,
&out_bias,
×tamp)) {
- set_node_bias(layer, &node, take(description), in_bias,
+ set_node_bias(layer, &node, description, in_bias,
/* relative = */ false,
/* out dir = */ false, timestamp);
- set_node_bias(layer, &node, take(description), out_bias,
+ set_node_bias(layer, &node, description, out_bias,
/* relative = */ false,
/* out dir = */ true, timestamp);
}
### tests/test_askrene.py
@@ -473,7 +473,6 @@ def test_node_bias_rpc(node_factory):
assert listlayers == {"layers": [expect]}
-@pytest.mark.xfail(strict=True)
def test_node_bias_persistence(node_factory):
"""Test node bias persistence."""
# remove xpay, since it creates a layer!Why this scored 59/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.