Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/raft_server.c
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +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.
* 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);
Expand Down
106 changes: 106 additions & 0 deletions tests/test_bug34_ae_response_null_deref.c
Original file line number Diff line number Diff line change
@@ -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 <stdbool.h>
#include <assert.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <stdint.h>
#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));
}