diff --git a/c/CHANGELOG.rst b/c/CHANGELOG.rst index e75b9e2bf1..70ead6e4ea 100644 --- a/c/CHANGELOG.rst +++ b/c/CHANGELOG.rst @@ -10,6 +10,11 @@ In development - Update to kastore 2.1.3 to include file security fix (:user:`jeromekelleher`, :pr:`3478`). +- Fix ``tsk_table_collection_link_ancestors`` accepting a sample or ancestor ID + equal to the number of nodes, which indexed node-sized arrays one past their + end. Such IDs now return ``TSK_ERR_NODE_OUT_OF_BOUNDS``. + (:user:`asystemoffields`, :issue:`3484`, :pr:`3492`) + -------------------- [1.3.1] - 2026-03-06 -------------------- diff --git a/c/tests/test_tables.c b/c/tests/test_tables.c index 7fca96563a..013c1d3eb1 100644 --- a/c/tests/test_tables.c +++ b/c/tests/test_tables.c @@ -7625,6 +7625,19 @@ test_link_ancestors_input_errors(void) &tables, samples, 2, ancestors, 2, 0, &result); CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_NODE_OUT_OF_BOUNDS); + /* Sample and ancestor IDs one past the last node */ + ancestors[0] = 4; + samples[0] = (tsk_id_t) tables.nodes.num_rows; + ret = tsk_table_collection_link_ancestors( + &tables, samples, 2, ancestors, 2, 0, &result); + CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_NODE_OUT_OF_BOUNDS); + + samples[0] = 0; + ancestors[0] = (tsk_id_t) tables.nodes.num_rows; + ret = tsk_table_collection_link_ancestors( + &tables, samples, 2, ancestors, 2, 0, &result); + CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_NODE_OUT_OF_BOUNDS); + /* Duplicate sample IDs */ ancestors[0] = 4; samples[0] = 1; diff --git a/c/tskit/tables.c b/c/tskit/tables.c index d673619141..c6db8e17a0 100644 --- a/c/tskit/tables.c +++ b/c/tskit/tables.c @@ -7905,7 +7905,7 @@ ancestor_mapper_init_samples(ancestor_mapper_t *self, tsk_id_t *samples) /* Go through the samples to check for errors. */ for (j = 0; j < self->num_samples; j++) { - if (samples[j] < 0 || samples[j] > (tsk_id_t) self->tables->nodes.num_rows) { + if (samples[j] < 0 || samples[j] >= (tsk_id_t) self->tables->nodes.num_rows) { ret = tsk_trace_error(TSK_ERR_NODE_OUT_OF_BOUNDS); goto out; } @@ -7932,7 +7932,8 @@ ancestor_mapper_init_ancestors(ancestor_mapper_t *self, tsk_id_t *ancestors) /* Go through the samples to check for errors. */ for (j = 0; j < self->num_ancestors; j++) { - if (ancestors[j] < 0 || ancestors[j] > (tsk_id_t) self->tables->nodes.num_rows) { + if (ancestors[j] < 0 + || ancestors[j] >= (tsk_id_t) self->tables->nodes.num_rows) { ret = tsk_trace_error(TSK_ERR_NODE_OUT_OF_BOUNDS); goto out; } diff --git a/python/CHANGELOG.rst b/python/CHANGELOG.rst index 2af4a8654b..887360c8c5 100644 --- a/python/CHANGELOG.rst +++ b/python/CHANGELOG.rst @@ -22,6 +22,9 @@ - Update to kastore C version 2.1.3 to include file security fix (:user:`jeromekelleher`, :pr:`3478`). +- ``link_ancestors`` now raises ``LibraryError`` (node out of bounds) for a sample or + ancestor ID equal to the number of nodes, instead of reading and writing past the + end of its node arrays. (:user:`asystemoffields`, :issue:`3484`, :pr:`3492`) -------------------- [1.0.3] - 2026-05-14 diff --git a/python/tests/test_python_c.py b/python/tests/test_python_c.py index aba3bc54cb..c594c7a38f 100644 --- a/python/tests/test_python_c.py +++ b/python/tests/test_python_c.py @@ -390,11 +390,16 @@ def test_link_ancestors_bad_args(self): tc.link_ancestors(samples=[0, 1], ancestors=[11, -1]) with pytest.raises(_tskit.LibraryError): tc.link_ancestors(samples=[0, -1], ancestors=[11]) + # One past the last node ID is out of bounds too + with pytest.raises(_tskit.LibraryError, match="out of bounds"): + tc.link_ancestors(samples=[0, tc.nodes.num_rows], ancestors=[11]) + with pytest.raises(_tskit.LibraryError, match="out of bounds"): + tc.link_ancestors(samples=[0, 1], ancestors=[tc.nodes.num_rows]) def test_link_ancestors(self): ts = msprime.simulate(2, random_seed=1) tc = ts.dump_tables()._ll_tables - edges = tc.link_ancestors([0, 1], [3]) + edges = tc.link_ancestors([0, 1], [2]) assert isinstance(edges, _tskit.EdgeTable) del edges assert tc.edges.num_rows == 2