Fix security issues

c/CHANGELOG.rst

@@ -10,6 +10,23 @@ In development
 - Update to kastore 2.1.3 to include file security fix (:user:`jeromekelleher`,
   :pr:`3478`).
 
+**Bugfixes**
+
+- Fix off-by-one node ID bounds check in ``tsk_table_collection_link_ancestors``
+  which allowed out-of-bounds memory access (:user:`jeromekelleher`, :issue:`3484`).
+
+- Fix off-by-one node ID bounds check in ``tsk_table_collection_ibd_within`` and
+  ``tsk_table_collection_ibd_between`` which allowed out-of-bounds memory access
+  (:user:`jeromekelleher`, :issue:`3485`).
+
+- ``tsk_table_collection_delete_older`` now checks table integrity before
+  modifying any tables, avoiding out-of-bounds memory access for invalid node
+  or mutation references (:user:`jeromekelleher`, :issue:`3486`).
+
+- Check parameters before allocating memory in
+  ``tsk_treeseq_genealogical_nearest_neighbours``, avoiding a very large
+  allocation request for invalid numbers of reference sets (:user:`jeromekelleher`).
+
 --------------------
 [1.3.1] - 2026-03-06
 --------------------

c/tests/test_tables.c

@@ -1264,7 +1264,7 @@ test_node_table(void)
     const char *example = "An example of metadata schema with unicode 🎄🌳🌴🌲🎋";
     tsk_size_t example_length = (tsk_size_t) strlen(example);
     const char *example2 = "A different example 🎄🌳🌴🌲🎋";
-    tsk_size_t example2_length = (tsk_size_t) strlen(example);
+    tsk_size_t example2_length = (tsk_size_t) strlen(example2);
     tsk_node_table_set_metadata_schema(&table, example, example_length);
     CU_ASSERT_EQUAL(table.metadata_schema_length, example_length);
     CU_ASSERT_EQUAL(tsk_memcmp(table.metadata_schema, example, example_length), 0);
@@ -1963,7 +1963,7 @@ test_edge_table_with_options(tsk_flags_t options)
     const char *example = "An example of metadata schema with unicode 🎄🌳🌴🌲🎋";
     tsk_size_t example_length = (tsk_size_t) strlen(example);
     const char *example2 = "A different example 🎄🌳🌴🌲🎋";
-    tsk_size_t example2_length = (tsk_size_t) strlen(example);
+    tsk_size_t example2_length = (tsk_size_t) strlen(example2);
     ret = tsk_edge_table_set_metadata_schema(&table, example, example_length);
     CU_ASSERT_EQUAL_FATAL(ret, 0);
     CU_ASSERT_EQUAL(table.metadata_schema_length, example_length);
@@ -2998,7 +2998,7 @@ test_site_table(void)
     const char *example = "An example of metadata schema with unicode 🎄🌳🌴🌲🎋";
     tsk_size_t example_length = (tsk_size_t) strlen(example);
     const char *example2 = "A different example 🎄🌳🌴🌲🎋";
-    tsk_size_t example2_length = (tsk_size_t) strlen(example);
+    tsk_size_t example2_length = (tsk_size_t) strlen(example2);
     tsk_site_table_set_metadata_schema(&table, example, example_length);
     CU_ASSERT_EQUAL(table.metadata_schema_length, example_length);
     CU_ASSERT_EQUAL(tsk_memcmp(table.metadata_schema, example, example_length), 0);
@@ -3726,7 +3726,7 @@ test_mutation_table(void)
     const char *example = "An example of metadata schema with unicode 🎄🌳🌴🌲🎋";
     tsk_size_t example_length = (tsk_size_t) strlen(example);
     const char *example2 = "A different example 🎄🌳🌴🌲🎋";
-    tsk_size_t example2_length = (tsk_size_t) strlen(example);
+    tsk_size_t example2_length = (tsk_size_t) strlen(example2);
     tsk_mutation_table_set_metadata_schema(&table, example, example_length);
     CU_ASSERT_EQUAL(table.metadata_schema_length, example_length);
     CU_ASSERT_EQUAL(tsk_memcmp(table.metadata_schema, example, example_length), 0);
@@ -4568,7 +4568,7 @@ test_migration_table(void)
     const char *example = "An example of metadata schema with unicode 🎄🌳🌴🌲🎋";
     tsk_size_t example_length = (tsk_size_t) strlen(example);
     const char *example2 = "A different example 🎄🌳🌴🌲🎋";
-    tsk_size_t example2_length = (tsk_size_t) strlen(example);
+    tsk_size_t example2_length = (tsk_size_t) strlen(example2);
     tsk_migration_table_set_metadata_schema(&table, example, example_length);
     CU_ASSERT_EQUAL(table.metadata_schema_length, example_length);
     CU_ASSERT_EQUAL(tsk_memcmp(table.metadata_schema, example, example_length), 0);
@@ -5365,7 +5365,7 @@ test_individual_table(void)
     const char *example = "An example of metadata schema with unicode 🎄🌳🌴🌲🎋";
     tsk_size_t example_length = (tsk_size_t) strlen(example);
     const char *example2 = "A different example 🎄🌳🌴🌲🎋";
-    tsk_size_t example2_length = (tsk_size_t) strlen(example);
+    tsk_size_t example2_length = (tsk_size_t) strlen(example2);
     tsk_individual_table_set_metadata_schema(&table, example, example_length);
     CU_ASSERT_EQUAL(table.metadata_schema_length, example_length);
     CU_ASSERT_EQUAL(tsk_memcmp(table.metadata_schema, example, example_length), 0);
@@ -6072,7 +6072,7 @@ test_population_table(void)
     const char *example = "An example of metadata schema with unicode 🎄🌳🌴🌲🎋";
     tsk_size_t example_length = (tsk_size_t) strlen(example);
     const char *example2 = "A different example 🎄🌳🌴🌲🎋";
-    tsk_size_t example2_length = (tsk_size_t) strlen(example);
+    tsk_size_t example2_length = (tsk_size_t) strlen(example2);
     tsk_population_table_set_metadata_schema(&table, example, example_length);
     CU_ASSERT_EQUAL(table.metadata_schema_length, example_length);
     CU_ASSERT_EQUAL(tsk_memcmp(table.metadata_schema, example, example_length), 0);
@@ -7618,13 +7618,23 @@ test_link_ancestors_input_errors(void)
         &tables, samples, 2, ancestors, 2, 0, &result);
     CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_NODE_OUT_OF_BOUNDS);
 
+    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);
+
     /* Bad ancestor IDs */
     samples[0] = 0;
     ancestors[0] = -1;
     ret = tsk_table_collection_link_ancestors(
         &tables, samples, 2, ancestors, 2, 0, &result);
     CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_NODE_OUT_OF_BOUNDS);
 
+    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;
@@ -8384,6 +8394,7 @@ test_ibd_segments_errors(void)
     tsk_id_t samples[] = { 0, 1, 2 };
     tsk_id_t duplicate_samples[] = { 0, 1, 0 };
     tsk_id_t samples2[] = { -1, 1 };
+    tsk_id_t samples3[] = { 0, 1, 2 };
     tsk_size_t sample_set_sizes[] = { 3 };
     tsk_identity_segments_t result;
     tsk_identity_segment_list_t *list;
@@ -8404,6 +8415,17 @@ test_ibd_segments_errors(void)
     CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_NODE_OUT_OF_BOUNDS);
     tsk_identity_segments_free(&result);
 
+    samples3[0] = (tsk_id_t) tables.nodes.num_rows;
+    ret = tsk_table_collection_ibd_within(
+        &tables, &result, samples3, 3, 0.0, DBL_MAX, TSK_IBD_STORE_SEGMENTS);
+    CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_NODE_OUT_OF_BOUNDS);
+    tsk_identity_segments_free(&result);
+
+    ret = tsk_table_collection_ibd_between(&tables, &result, 1, sample_set_sizes,
+        samples3, 0.0, DBL_MAX, TSK_IBD_STORE_SEGMENTS);
+    CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_NODE_OUT_OF_BOUNDS);
+    tsk_identity_segments_free(&result);
+
     // Bad length or time
     ret = tsk_table_collection_ibd_within(&tables, &result, samples, 2, 0.0, -1, 0);
     CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_BAD_PARAM_VALUE);
@@ -11886,6 +11908,60 @@ test_table_collection_delete_older(void)
     tsk_treeseq_free(&ts);
 }
 
+static void
+test_table_collection_delete_older_errors(void)
+{
+    int ret;
+    tsk_treeseq_t ts;
+    tsk_table_collection_t t, copy;
+
+    const char *mutations = "0      2   1   -1\n"
+                            "0      2   0   0\n"
+                            "1      0   1   -1\n"
+                            "2      5   1   -1\n";
+
+    tsk_treeseq_from_text(&ts, 10, paper_ex_nodes, paper_ex_edges, NULL, paper_ex_sites,
+        mutations, paper_ex_individuals, NULL, 0);
+    ret = tsk_treeseq_copy_tables(&ts, &t, 0);
+    CU_ASSERT_EQUAL_FATAL(ret, 0);
+    tsk_treeseq_free(&ts);
+
+    /* Bad edge parent */
+    t.edges.parent[0] = (tsk_id_t) t.nodes.num_rows;
+    ret = tsk_table_collection_copy(&t, &copy, 0);
+    CU_ASSERT_EQUAL_FATAL(ret, 0);
+    ret = tsk_table_collection_delete_older(&t, 0.09, 0);
+    CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_NODE_OUT_OF_BOUNDS);
+    CU_ASSERT_TRUE(tsk_table_collection_equals(&t, &copy, 0));
+    t.edges.parent[0] = 4;
+    tsk_table_collection_free(&copy);
+
+    /* Bad mutation node */
+    t.mutations.node[0] = (tsk_id_t) t.nodes.num_rows;
+    ret = tsk_table_collection_copy(&t, &copy, 0);
+    CU_ASSERT_EQUAL_FATAL(ret, 0);
+    ret = tsk_table_collection_delete_older(&t, 0.09, 0);
+    CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_NODE_OUT_OF_BOUNDS);
+    CU_ASSERT_TRUE(tsk_table_collection_equals(&t, &copy, 0));
+    t.mutations.node[0] = 2;
+    tsk_table_collection_free(&copy);
+
+    /* Bad mutation parent */
+    t.mutations.parent[1] = (tsk_id_t) t.mutations.num_rows;
+    ret = tsk_table_collection_copy(&t, &copy, 0);
+    CU_ASSERT_EQUAL_FATAL(ret, 0);
+    ret = tsk_table_collection_delete_older(&t, 0.09, 0);
+    CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_MUTATION_OUT_OF_BOUNDS);
+    CU_ASSERT_TRUE(tsk_table_collection_equals(&t, &copy, 0));
+    t.mutations.parent[1] = 0;
+    tsk_table_collection_free(&copy);
+
+    ret = tsk_table_collection_delete_older(&t, 0.09, 0);
+    CU_ASSERT_EQUAL_FATAL(ret, 0);
+
+    tsk_table_collection_free(&t);
+}
+
 int
 main(int argc, char **argv)
 {
@@ -12030,6 +12106,8 @@ main(int argc, char **argv)
         { "test_table_collection_takeset_indexes",
             test_table_collection_takeset_indexes },
         { "test_table_collection_delete_older", test_table_collection_delete_older },
+        { "test_table_collection_delete_older_errors",
+            test_table_collection_delete_older_errors },
         { NULL, NULL },
     };
 

c/tests/test_trees.c

@@ -7873,7 +7873,7 @@ test_zero_edges(void)
                             "1    1     1\n";
     tsk_treeseq_t ts, tss;
     tsk_tree_t t;
-    tsk_id_t samples, node_map;
+    tsk_id_t samples, node_map[2];
     const tsk_id_t z = TSK_NULL;
     tsk_id_t parents[] = {
         z,
@@ -7921,9 +7921,12 @@ test_zero_edges(void)
     tsk_tree_print_state(&t, _devnull);
     tsk_tree_free(&t);
 
-    /* We give pointers ot samples and node_map here as they must be non null */
-    ret = tsk_treeseq_simplify(&ts, &samples, 0, 0, &tss, &node_map);
+    /* We give a pointer to samples here as it must be non null; node_map must
+     * have space for all input nodes */
+    ret = tsk_treeseq_simplify(&ts, &samples, 0, 0, &tss, node_map);
     CU_ASSERT_EQUAL_FATAL(ret, 0);
+    CU_ASSERT_EQUAL(node_map[0], TSK_NULL);
+    CU_ASSERT_EQUAL(node_map[1], TSK_NULL);
     CU_ASSERT_EQUAL(tsk_treeseq_get_num_samples(&tss), 0);
     CU_ASSERT_EQUAL(tsk_treeseq_get_sequence_length(&tss), 2.0);
     CU_ASSERT_EQUAL(tsk_treeseq_get_num_nodes(&tss), 0);

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;
         }
@@ -8632,7 +8633,7 @@ tsk_ibd_finder_init_samples_from_set(
     for (j = 0; j < num_samples; j++) {
         u = samples[j];
 
-        if (u < 0 || u > (tsk_id_t) self->tables->nodes.num_rows) {
+        if (u < 0 || u >= (tsk_id_t) self->tables->nodes.num_rows) {
             ret = tsk_trace_error(TSK_ERR_NODE_OUT_OF_BOUNDS);
             goto out;
         }
@@ -8886,7 +8887,7 @@ tsk_ibd_finder_init_between(tsk_ibd_finder_t *self, tsk_size_t num_sample_sets,
     for (j = 0; j < num_sample_sets; j++) {
         for (k = 0; k < sample_set_sizes[j]; k++) {
             u = sample_sets[index];
-            if (u < 0 || u > (tsk_id_t) self->tables->nodes.num_rows) {
+            if (u < 0 || u >= (tsk_id_t) self->tables->nodes.num_rows) {
                 ret = tsk_trace_error(TSK_ERR_NODE_OUT_OF_BOUNDS);
                 goto out;
             }
@@ -12629,6 +12630,10 @@ tsk_table_collection_delete_older(
     memset(&mutations, 0, sizeof(mutations));
     memset(&migrations, 0, sizeof(migrations));
 
+    ret = (int) tsk_table_collection_check_integrity(self, 0);
+    if (ret != 0) {
+        goto out;
+    }
     ret = tsk_edge_table_copy(&self->edges, &edges, 0);
     if (ret != 0) {
         goto out;

c/tskit/trees.c

@@ -911,12 +911,10 @@ tsk_treeseq_genealogical_nearest_neighbours(const tsk_treeseq_t *self,
     const double sequence_length = self->tables->sequence_length;
     tsk_id_t tj, tk, h;
     double left, right, *A_row, scale, tree_length;
-    tsk_id_t *restrict parent = tsk_malloc(num_nodes * sizeof(*parent));
-    double *restrict length = tsk_calloc(num_focal, sizeof(*length));
-    uint32_t *restrict ref_count
-        = tsk_calloc(((tsk_size_t) K) * num_nodes, sizeof(*ref_count));
-    int16_t *restrict reference_set_map
-        = tsk_malloc(num_nodes * sizeof(*reference_set_map));
+    tsk_id_t *restrict parent = NULL;
+    double *restrict length = NULL;
+    uint32_t *restrict ref_count = NULL;
+    int16_t *restrict reference_set_map = NULL;
     uint32_t *restrict row = NULL;
     uint32_t *restrict child_row = NULL;
     uint32_t total, delta;
@@ -927,6 +925,10 @@ tsk_treeseq_genealogical_nearest_neighbours(const tsk_treeseq_t *self,
         ret = tsk_trace_error(TSK_ERR_BAD_PARAM_VALUE);
         goto out;
     }
+    parent = tsk_malloc(num_nodes * sizeof(*parent));
+    length = tsk_calloc(num_focal, sizeof(*length));
+    ref_count = tsk_calloc(((tsk_size_t) K) * num_nodes, sizeof(*ref_count));
+    reference_set_map = tsk_malloc(num_nodes * sizeof(*reference_set_map));
     if (parent == NULL || ref_count == NULL || reference_set_map == NULL
         || length == NULL) {
         ret = tsk_trace_error(TSK_ERR_NO_MEMORY);

python/CHANGELOG.rst

@@ -22,6 +22,9 @@
 
 - Update to kastore C version 2.1.3 to include file security fix (:user:`jeromekelleher`, 
   :pr:`3478`).
+- ``TableCollection.delete_older`` now checks table integrity first, raising an
+  error for invalid node or mutation references instead of reading out of bounds
+  (:user:`jeromekelleher`, :issue:`3486`).
 
 --------------------
 [1.0.3] - 2026-05-14

python/tests/test_python_c.py

@@ -394,11 +394,20 @@ def test_link_ancestors_bad_args(self):
     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
 
+    @pytest.mark.parametrize(
+        ["samples", "ancestors"], [([0, 1], [3]), ([0, 3], [2]), ([0, 1], [-1])]
+    )
+    def test_link_ancestors_bad_node(self, samples, ancestors):
+        ts = msprime.simulate(2, random_seed=1)
+        tc = ts.dump_tables()._ll_tables
+        with pytest.raises(_tskit.LibraryError, match="TSK_ERR_NODE_OUT_OF_BOUNDS"):
+            tc.link_ancestors(samples, ancestors)
+
     def test_subset_bad_args(self):
         ts = msprime.simulate(10, random_seed=1)
         tc = ts.dump_tables()._ll_tables

python/tests/test_table_transforms.py

@@ -148,6 +148,36 @@ def test_t2(self, time):
         tables.delete_older(time)
         tables.assert_equals(before, ignore_provenance=True)
 
+    @pytest.mark.parametrize("bad_node", [5, 2**30])
+    def test_bad_edge_parent(self, bad_node):
+        tables = self.tables()
+        parent = tables.edges.parent
+        parent[0] = bad_node
+        tables.edges.parent = parent
+        before = tables.copy()
+        with pytest.raises(tskit.LibraryError, match="TSK_ERR_NODE_OUT_OF_BOUNDS"):
+            tables.delete_older(1)
+        tables.assert_equals(before)
+
+    @pytest.mark.parametrize("bad_node", [5, 2**30])
+    def test_bad_mutation_node(self, bad_node):
+        tables = self.tables()
+        tables.sites.add_row(0.5, "A")
+        tables.mutations.add_row(site=0, node=bad_node, derived_state="T")
+        before = tables.copy()
+        with pytest.raises(tskit.LibraryError, match="TSK_ERR_NODE_OUT_OF_BOUNDS"):
+            tables.delete_older(1)
+        tables.assert_equals(before)
+
+    def test_bad_mutation_parent(self):
+        tables = self.tables()
+        tables.sites.add_row(0.5, "A")
+        tables.mutations.add_row(site=0, node=0, derived_state="T", parent=2**30)
+        before = tables.copy()
+        with pytest.raises(tskit.LibraryError, match="TSK_ERR_MUTATION_OUT_OF_BOUNDS"):
+            tables.delete_older(1)
+        tables.assert_equals(before)
+
 
 class TestDeleteOlderSimpleTreeMutationExamples:
     def test_single_mutation_no_time(self):