authorgravatar for Validark@pm.meNiles Salter <Validark@pm.me> 2023-06-20 21:05:12-06:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2023-06-22 02:03:53-07:00
log82470d4f892af2efa64acd58ab3188fa917ace0c
tree86fc03ba6899051e78a57d3c3ebdb3004ea5044f
parentff5850183eee854fdfe0d3f7b7242b9ff56c2116

[priority_dequeue] Fix out-of-bounds access

This makes it so `first_child_index` will not be accessed when it is equal to `self.len`. (i.e. `self.items[self.len]` will not happen) The access itself was "safe" (as in, `self.len < self.items.len`) because we were only calling `doSiftDown` in the case where there was a stale value at `self.items[self.len]`. However, it is still technically a bug, and can manifest by an unnecessary comparison of a value to a copy of itself.

1 files changed, 23 insertions(+), 1 deletions(-)

lib/std/priority_dequeue.zig+23-1
...@@ -230,7 +230,7 @@ pub fn PriorityDequeue(comptime T: type, comptime Context: type, comptime compar...@@ -230,7 +230,7 @@ pub fn PriorityDequeue(comptime T: type, comptime Context: type, comptime compar
230 } else {230 } else {
231 // The children or grandchildren are the last layer231 // The children or grandchildren are the last layer
232 const first_child_index = firstChildIndex(index);232 const first_child_index = firstChildIndex(index);
233 if (first_child_index > self.len) return;233 if (first_child_index >= self.len) return;
234234
235 const best_descendent = self.bestDescendent(first_child_index, first_grandchild_index, target_order);235 const best_descendent = self.bestDescendent(first_child_index, first_grandchild_index, target_order);
236236
...@@ -1002,3 +1002,25 @@ test "std.PriorityDequeue: add and remove" {...@@ -1002,3 +1002,25 @@ test "std.PriorityDequeue: add and remove" {
1002 try expectEqual(@as(usize, 2), queue.removeMax());1002 try expectEqual(@as(usize, 2), queue.removeMax());
1003 try expectEqual(@as(usize, 1), queue.removeMin());1003 try expectEqual(@as(usize, 1), queue.removeMin());
1004}1004}
1005
1006var all_cmps_unique = true;
1007
1008test "std.PriorityDeque: don't compare a value to a copy of itself" {
1009 var depq = PriorityDequeue(u32, void, struct {
1010 fn uniqueLessThan(_: void, a: u32, b: u32) Order {
1011 all_cmps_unique = all_cmps_unique and (a != b);
1012 return std.math.order(a, b);
1013 }
1014 }.uniqueLessThan).init(testing.allocator, {});
1015 defer depq.deinit();
1016
1017 try depq.add(1);
1018 try depq.add(2);
1019 try depq.add(3);
1020 try depq.add(4);
1021 try depq.add(5);
1022 try depq.add(6);
1023
1024 _ = depq.removeIndex(2);
1025 try expectEqual(all_cmps_unique, true);
1026}