search

rust-scraper / ego-tree

Vec-backed ID-tree
https://docs.rs/ego-tree
ISC License
45 stars 16 forks source link

Fix out-of-bounds access possibility in safe code. #18

Closed SimonSapin closed 5 years ago

SimonSapin commented 5 years ago

With the id and tree fields of NodeRef and NodeMut being public, it was possible to assign to them. For example, it was possible to build a NodeMut for large ID/index in a small tree/Vec. Since some APIs use unchecked indexing, this would let users of this library cause out-of-bounds access in a Vec without writing unsafe code themselves.

This commit fixes that issue by making the fields private and instead providing read-only access via accessor methods. Now the fields can only be set by the ego-tree crate, which can make sure to only ever use an ID that is in-bounds for a given tree.

causal-agent commented 5 years ago

Yikes. If I remember correctly this used to be the API but at some point I switched to public fields thinking that NodeId being opaque was enough.

SimonSapin commented 5 years ago

I’ve added an intermediate commit with a SemVer-compatible version that keeps the fields public but deprecates them. Publishing both commits might be best to do now.

causal-agent commented 5 years ago

Great idea. Thanks

causal-agent commented 5 years ago

0.5.1 and 0.6.0 published