Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add advisory for UB in crossbeam-channel 0.4.3 #425

Merged
merged 3 commits into from
Oct 11, 2020

Conversation

taiki-e
Copy link
Contributor

@taiki-e taiki-e commented Oct 11, 2020

The affected version of this crate's the bounded channel incorrectly assumes that Vec::from_iter has allocated capacity that same as the number of iterator elements. Vec::from_iter does not actually guarantee that and may allocate extra memory. The destructor of the bounded channel reconstructs Vec from the raw pointer based on the incorrect assumes described above. This is unsound and causing deallocation with the incorrect capacity when Vec::from_iter has allocated different sizes with the number of iterator elements.

Refs: crossbeam-rs/crossbeam#533, crossbeam-rs/crossbeam#539 (Both issues are mainly saying about crossbeam-queue, but crossbeam-queue including this UB, has not been released and only crossbeam-channel is affected.)

@Shnatsel
Copy link
Member

Shnatsel commented Oct 11, 2020

Thanks for the report!

Is this purely theoretical at this point, or has deallocation of the incorrect size has actually been observed in practice? If it's the latter, I'd drop informational = "unsound".

Last time I checked from_iter never over-allocated in practice, but that was over a year ago and there have been some optimizations for small vector sizes, like the first allocation was always having size 4.

@taiki-e
Copy link
Contributor Author

taiki-e commented Oct 11, 2020

Is this purely theoretical at this point, or has deallocation of the incorrect size has actually been observed in practice? If it's the latter, I'd drop informational = "unsound".

It has actually been observed in practice (https://twitter.com/khuey_/status/1311641831201857537, nervosnetwork/ckb#2244), so I'll drop informational field.

Last time I checked from_iter never over-allocated in practice, but that was over a year ago and there have been some optimizations for small vector sizes, like the first allocation was always having size 4.

If the number of elements is small, it seems that 0-7 extra capacity is allocated depending on the size of the elements.
(playground)

@Shnatsel
Copy link
Member

Merging. Thanks!

@Shnatsel Shnatsel merged commit 75a51cb into rustsec:master Oct 11, 2020
@taiki-e taiki-e deleted the crossbeam-533 branch October 11, 2020 13:00
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 12, 2020
CjS77 added a commit to tari-project/broadcast_channel that referenced this pull request Jun 7, 2022
Impact
The affected version of this crate's the bounded channel incorrectly assumes that Vec::from_iter has allocated capacity that same as the number of iterator elements. Vec::from_iter does not actually guarantee that and may allocate extra memory. The destructor of the bounded channel reconstructs Vec from the raw pointer based on the incorrect assumes described above. This is unsound and causing deallocation with the incorrect capacity when Vec::from_iter has allocated different sizes with the number of iterator elements.

Patches
This has been fixed in crossbeam-channel 0.4.4.

We recommend users to upgrade to 0.4.4.

References
See crossbeam-rs/crossbeam#533, crossbeam-rs/crossbeam#539, and rustsec/advisory-db#425 for more details.
CjS77 added a commit to tari-project/broadcast_channel that referenced this pull request Jun 7, 2022
Impact
The affected version of this crate's the bounded channel incorrectly assumes that Vec::from_iter has allocated capacity that same as the number of iterator elements. Vec::from_iter does not actually guarantee that and may allocate extra memory. The destructor of the bounded channel reconstructs Vec from the raw pointer based on the incorrect assumes described above. This is unsound and causing deallocation with the incorrect capacity when Vec::from_iter has allocated different sizes with the number of iterator elements.

Patches
This has been fixed in crossbeam-channel 0.4.4.

We recommend users to upgrade to 0.4.4.

References
See crossbeam-rs/crossbeam#533, crossbeam-rs/crossbeam#539, and rustsec/advisory-db#425 for more details.
CjS77 added a commit to tari-project/broadcast_channel that referenced this pull request Jun 7, 2022
Impact
The affected version of this crate's the bounded channel incorrectly assumes that Vec::from_iter has allocated capacity that same as the number of iterator elements. Vec::from_iter does not actually guarantee that and may allocate extra memory. The destructor of the bounded channel reconstructs Vec from the raw pointer based on the incorrect assumes described above. This is unsound and causing deallocation with the incorrect capacity when Vec::from_iter has allocated different sizes with the number of iterator elements.

Patches
This has been fixed in crossbeam-channel 0.4.4.

We recommend users to upgrade to 0.4.4.

References
See crossbeam-rs/crossbeam#533, crossbeam-rs/crossbeam#539, and rustsec/advisory-db#425 for more details.
CjS77 added a commit to tari-project/broadcast_channel that referenced this pull request Jun 7, 2022
Impact
The affected version of this crate's the bounded channel incorrectly assumes that Vec::from_iter has allocated capacity that same as the number of iterator elements. Vec::from_iter does not actually guarantee that and may allocate extra memory. The destructor of the bounded channel reconstructs Vec from the raw pointer based on the incorrect assumes described above. This is unsound and causing deallocation with the incorrect capacity when Vec::from_iter has allocated different sizes with the number of iterator elements.

Patches
This has been fixed in crossbeam-channel 0.4.4.

We recommend users to upgrade to 0.4.4.

References
See crossbeam-rs/crossbeam#533, crossbeam-rs/crossbeam#539, and rustsec/advisory-db#425 for more details.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants