Skip to content

Soundness issue in Zip::next() specialization #81740

Closed

Description

#[inline]
fn next(&mut self) -> Option<(A::Item, B::Item)> {
if self.index < self.len {
let i = self.index;
self.index += 1;
// SAFETY: `i` is smaller than `self.len`, thus smaller than `self.a.len()` and `self.b.len()`
unsafe {
Some((self.a.__iterator_get_unchecked(i), self.b.__iterator_get_unchecked(i)))
}
} else if A::may_have_side_effect() && self.index < self.a.size() {
// match the base implementation's potential side effects
// SAFETY: we just checked that `self.index` < `self.a.len()`
unsafe {
self.a.__iterator_get_unchecked(self.index);
}
self.index += 1;
None
} else {
None
}
}

/// 2. If `self: !Clone`, then `get_unchecked` is never called with the same
/// index on `self` more than once.

There is a panic safety issue in Zip::next() that allows to call __iterator_get_unchecked() to the same index twice. __iterator_get_unchecked() is called at line 204 and the index is updated at line 206. If line 204 panics, the index is not updated and the subsequent next() call will use the same index for __iterator_get_unchecked(). This violates the second safety requirement of TrustedRandomAccess.

Here is a playground link that demonstrates creating two mutable references to the same memory location without using unsafe Rust.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Labels

    A-iteratorsArea: IteratorsC-bugCategory: This is a bug.I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessP-highHigh priorityT-libsRelevant to the library team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions