servo/rust-smallvec

`SmallVec::insert_many` is unsound

eira-fransham opened this issue · 5 comments

eira-fransham commented

Gist here. A solution to this would be to set len = index before iterating. Obviously this would cause leaks but we're already leaking data.

(Sorry for the very verbose explanation in the linked gist but I wanted to link it as part of an article whose audience might not immediately understand the problem).

mbrubeck commented

Good catch. The proposed solution sounds good to me.

Shnatsel commented

Double free is an exploitable security vulnerability. Once the fix is released, please add this issue to the Rust security advisory database.

I would also urge you to publish a fixed version ASAP. Despite the fact that the double free only happens on panic, this bug still can be exploited in a multi-threaded application that catches panics. Also, panics are really easy to trigger in many popular crates.

  • 👍3
  • ❤️4
mbrubeck commented

Submitted a fix in #103.

  • 👍2
  • 🎉2
Shnatsel commented

Would you consider yanking vulnerable versions from crates.io?

Existing dependencies on them will continue to work, but people won't be able to make new crates depending on vulnerable versions by following e.g. a tutorial that was not updated following this advisory.

Edit: nevermind, already answered here