From 23e755cc853a870c0e7c760c9cf9dd4cf84fdf0f Mon Sep 17 00:00:00 2001 From: professor-moody Date: Fri, 17 Jul 2026 18:55:22 -0500 Subject: [PATCH 1/2] Bound file-controlled node index against _n_nodes in the query tree walk _get_all_nns pushes a split node's children[0]/children[1] (read from the index file) back onto the search queue and dereferences them on the next hop via _get(i) = _nodes + _s*i, with no check that i is within [0, _n_nodes). A crafted .ann whose split node has an out-of-range child index causes an out-of-bounds read when loaded and queried. Add the bound before the dereference (this is the // TODO: handle OOB noted above). --- src/annoylib.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/annoylib.h b/src/annoylib.h index 657977cb..28cf8408 100644 --- a/src/annoylib.h +++ b/src/annoylib.h @@ -1467,6 +1467,7 @@ template= _n_nodes) continue; // bound the file-controlled node index before deref if (nd->n_descendants == 1 && i < _n_items) { nns.push_back(i); } else if (nd->n_descendants <= _K) { From 35b161f5f328b7fd1247aa5e5f4f790e3b048a86 Mon Sep 17 00:00:00 2001 From: Nathan Keys Date: Fri, 31 Jul 2026 10:40:16 -0500 Subject: [PATCH 2/2] Bound the leaf children copied into nns, and guard before forming the pointer Testing the first commit against a wider set of tampered indexes showed it did not close the reported out-of-bounds read. It only bounds indices that travel through the priority queue. A leaf node's children[] array is copied verbatim into nns and then dereferenced in the dedup loop, which never sees the loop guard, so a crafted index still faults there: AddressSanitizer: BUS on unknown address, READ #0 _get_all_nns annoylib.h:1493 #1 get_nns_by_vector annoylib.h:1235 Three changes: 1. Move the existing guard above _get(i). The index has to be bounded before it is turned into a pointer, not only before that pointer is read. 2. Reject a negative n_descendants on the leaf branch before using it as the length of the copy. Zero stays legal, an empty leaf is a normal thing. 3. Bound the values copied out of a leaf's children[] where the dedup loop dereferences them. This is the site that actually faulted. Also bounds the item index in get_nns_by_item, which carried a TODO for it. Verified on a 200 item, 4 tree index: sweeping single 4 byte edits across every leaf-array node crashed the previous version and crashes nothing here, over three full sweeps. Results are byte identical to unpatched annoy across 108 legitimate index configurations, dimensions 2 to 128, 1 to 1000 items, 1 to 10 trees, including the single item and two item cases. --- src/annoylib.h | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/annoylib.h b/src/annoylib.h index 28cf8408..b053a69e 100644 --- a/src/annoylib.h +++ b/src/annoylib.h @@ -1226,7 +1226,8 @@ template* result, vector* distances) const { - // TODO: handle OOB + if (item < 0 || item >= _n_nodes) + return; const Node* m = _get(item); _get_all_nns(m->v, n, search_k, result, distances); } @@ -1465,12 +1466,19 @@ template& top = q.top(); T d = top.first; S i = top.second; - Node* nd = _get(i); q.pop(); - if (i < 0 || i >= _n_nodes) continue; // bound the file-controlled node index before deref + // i comes from the file (a child index), so it must be bounded before it + // is turned into a pointer, not merely before that pointer is read. + if (i < 0 || i >= _n_nodes) + continue; + Node* nd = _get(i); if (nd->n_descendants == 1 && i < _n_items) { nns.push_back(i); } else if (nd->n_descendants <= _K) { + // n_descendants is file controlled here too; zero is legitimate (an + // empty leaf), negative is not, and would run the copy backwards. + if (nd->n_descendants < 0) + continue; const S* dst = nd->children; nns.insert(nns.end(), dst, &dst[nd->n_descendants]); } else { @@ -1490,6 +1498,10 @@ template= _n_nodes) + continue; if (_get(j)->n_descendants == 1) // This is only to guard a really obscure case, #284 nns_dist.push_back(make_pair(D::distance(v_node, _get(j), _f), j)); }