AcademySoftwareFoundation / openvdb

OpenVDB - Sparse volume data structure and tools
http://www.openvdb.org/
Apache License 2.0
2.71k stars 660 forks source link

Prevent integer overflow in NodeManager and LeafManager #1794

Closed ghurstunither closed 2 weeks ago

ghurstunither commented 7 months ago

For large enough grids, leafCounts in initLeafArray() and nodeCounts in initNodeChildren() can overflow as std::vector<Index32>.

ghurstunither commented 6 months ago

Needs a change for the log

Is that something I add in this PR?

would be ideal if we could add a simple unit test that exceeds 32-bit integer.

I'm not sure how to do that without making a gigantic tree.

danrbailey commented 3 weeks ago

I think there's actually more to talk about here. The Tree and RootNode also make assumptions that the number of leaf nodes cannot exceed 2^32:

https://github.com/AcademySoftwareFoundation/openvdb/blob/master/openvdb/openvdb/tree/Tree.h#L116

The tree ones in particular are actually virtual functions, so if we want to bump this arbitrary limit, we should also change those methods as an ABI change.

ghurstunither commented 3 weeks ago

I think there's actually more to talk about here. The Tree and RootNode also make assumptions that the number of leaf nodes cannot exceed 2^32:

https://github.com/AcademySoftwareFoundation/openvdb/blob/master/openvdb/openvdb/tree/Tree.h#L116

The tree ones in particular are actually virtual functions, so if we want to bump this arbitrary limit, we should also change those methods as an ABI change.

Thanks for pointing this out. I think the following should return Index64:

The other methods seem fine to return Index32 if we assume we'll never exceed 2^32 for other nodes, which I think is reasonable.