From fe3a3230bffa01cd69e69c7ecad927ce890eedaf Mon Sep 17 00:00:00 2001 From: Qian-Cheng-nju Date: Wed, 25 Mar 2026 04:08:31 +0000 Subject: [PATCH 1/3] fix: add bounds check for current_idx in AE response handler --- src/raft_server.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/raft_server.c b/src/raft_server.c index aefa2184..a44367f4 100644 --- a/src/raft_server.c +++ b/src/raft_server.c @@ -343,7 +343,12 @@ int raft_recv_appendentries_response(raft_server_t* me_, if (r->current_idx <= match_idx) return 0; + /* If current_idx is beyond our log, it's a bogus or stale response. + * The assert below only fires in debug builds; in release builds the + * subsequent raft_get_entry_from_idx would return NULL and crash. */ assert(r->current_idx <= raft_get_current_idx(me_)); + if (raft_get_current_idx(me_) < r->current_idx) + return 0; raft_node_set_next_idx(node, r->current_idx + 1); raft_node_set_match_idx(node, r->current_idx); From f0a4f35b175e70b039e435e690b371c3797f1b34 Mon Sep 17 00:00:00 2001 From: Qian-Cheng-nju Date: Wed, 25 Mar 2026 04:21:13 +0000 Subject: [PATCH 2/3] fix: move bounds check before assert to prevent debug-build abort --- src/raft_server.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/raft_server.c b/src/raft_server.c index a44367f4..8dc3a1c9 100644 --- a/src/raft_server.c +++ b/src/raft_server.c @@ -344,12 +344,13 @@ int raft_recv_appendentries_response(raft_server_t* me_, return 0; /* If current_idx is beyond our log, it's a bogus or stale response. - * The assert below only fires in debug builds; in release builds the - * subsequent raft_get_entry_from_idx would return NULL and crash. */ - assert(r->current_idx <= raft_get_current_idx(me_)); + * Without this check, raft_get_entry_from_idx returns NULL and the + * subsequent ety->term dereferences NULL (crash in release builds). */ if (raft_get_current_idx(me_) < r->current_idx) return 0; + assert(r->current_idx <= raft_get_current_idx(me_)); + raft_node_set_next_idx(node, r->current_idx + 1); raft_node_set_match_idx(node, r->current_idx); From bf49aa2ad559e1eee1b3251659fc2f52397e13b2 Mon Sep 17 00:00:00 2001 From: Qian-Cheng-nju Date: Wed, 25 Mar 2026 04:21:18 +0000 Subject: [PATCH 3/3] test: add reproduction test for AE response bounds check --- tests/test_bug34_ae_response_null_deref.c | 106 ++++++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 tests/test_bug34_ae_response_null_deref.c diff --git a/tests/test_bug34_ae_response_null_deref.c b/tests/test_bug34_ae_response_null_deref.c new file mode 100644 index 00000000..d54c1b56 --- /dev/null +++ b/tests/test_bug34_ae_response_null_deref.c @@ -0,0 +1,106 @@ +/** + * Reproduction test for Bug #34: AE response current_idx out-of-bounds. + * + * Before the fix, if a follower sends an AE response with current_idx + * beyond the leader's log length, raft_get_entry_from_idx returns NULL + * and the subsequent ety->term dereferences NULL (crash in release builds, + * assert in debug builds). + * + * The fix adds a bounds check that silently drops the bogus response. + */ + +#include +#include +#include +#include +#include +#include +#include "CuTest.h" + +#include "raft.h" +#include "raft_log.h" +#include "raft_private.h" +#include "mock_send_functions.h" + +static int __persist_term( + raft_server_t* raft, + void *udata, + raft_term_t term, + int vote + ) +{ + return 0; +} + +static int __persist_vote( + raft_server_t* raft, + void *udata, + int vote + ) +{ + return 0; +} + +static int __log_offer( + raft_server_t* raft, + void *udata, + raft_entry_t *ety, + raft_index_t idx + ) +{ + return 0; +} + +/** + * If a follower sends an AE response with current_idx far beyond the + * leader's log, the leader should silently drop the message instead of + * crashing on a NULL entry dereference. + */ +void TestRaft_leader_recv_appendentries_response_drops_bogus_current_idx( + CuTest * tc) +{ + raft_cbs_t funcs = { + .persist_term = __persist_term, + .persist_vote = __persist_vote, + .send_appendentries = sender_appendentries, + .log_offer = __log_offer, + }; + + void *sender = sender_new(NULL); + void *r = raft_new(); + raft_add_node(r, NULL, 1, 1); + raft_add_node(r, NULL, 2, 0); + raft_set_callbacks(r, &funcs, sender); + + /* Become leader */ + raft_set_state(r, RAFT_STATE_LEADER); + raft_set_current_term(r, 1); + + /* Append one entry so the leader has a log of length 1 */ + msg_entry_t ety = {}; + ety.id = 1; + ety.type = RAFT_LOGTYPE_NORMAL; + msg_entry_response_t eresp; + raft_recv_entry(r, &ety, &eresp); + CuAssertIntEquals(tc, 1, raft_get_current_idx(r)); + + raft_node_t* node2 = raft_get_node(r, 2); + int orig_next_idx = raft_node_get_next_idx(node2); + + /* Fabricate an AE response with current_idx far beyond the log. + * Before the fix this would crash (NULL deref in release, + * assert failure in debug). */ + msg_appendentries_response_t aer; + aer.term = 1; + aer.success = 1; + aer.current_idx = 999; /* way beyond log length of 1 */ + aer.first_idx = 1; + + int e = raft_recv_appendentries_response(r, node2, &aer); + + /* The fix causes the function to return 0 (drop the message) */ + CuAssertIntEquals(tc, 0, e); + + /* next_idx should not have been advanced by the bogus response */ + CuAssertIntEquals(tc, orig_next_idx, raft_node_get_next_idx(node2)); +}