Skip to content

fix bugs with list / tuple iterator "nth" operations#6086

Merged
davidhewitt merged 6 commits into
PyO3:mainfrom
davidhewitt:nth-ops
Jun 2, 2026
Merged

fix bugs with list / tuple iterator "nth" operations#6086
davidhewitt merged 6 commits into
PyO3:mainfrom
davidhewitt:nth-ops

Conversation

@davidhewitt

@davidhewitt davidhewitt commented Jun 1, 2026

Copy link
Copy Markdown
Member

This PR fixes issues with tuple and list iterators:

  • nth / nth_back with large N could overflow usize and wrap, leading to out of bounds reads via get_unchecked
  • if N exceeds the number of remaining items in the iterator, nth / nth_back should exhaust the iterator - the existing implementations did not do this

This is primarily done by rewriting nth / nth_back to use checked_ mathematical operations to avoid the possible overflow.

I verified the advance_by / advance_back_by nightly operations were not affected by the same issue; they turned out to be fine, however I wrote them to use saturating_ operations to make it easier (for me) to read their implementations.

(Credit to Codex security scanning for the discovery of the out of bound reads; the failure to exhaust is my addition when scrutinising these methods.)

@davidhewitt davidhewitt mentioned this pull request Jun 1, 2026
@davidhewitt

Copy link
Copy Markdown
Member Author

I think this out-of-bounds read is worth filing to the RustSec advisory db. Unlike the other discoveries from Codex that we've handled as part of #6010, I think this one could easily manifest as a vulnerability in user code which could lie undetected until a malicious attacker passes a large N.

Reviewers, please let me know if you agree with me!

@ngoldbaum ngoldbaum left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good and probably fine to merge as-is but I spotted some issues.

Comment thread src/types/list.rs Outdated
Comment thread src/types/list.rs Outdated
@@ -0,0 +1,2 @@
- Fix possible out of bounds read in `BoundListIterator` and `BoundTupleIterator`'s `nth` and `nth_back` implementations.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't only nth_back a possible UB?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nth when the iterator has been advanced forwards could still wrap around to the front of the collection. This isn't UB in the same way as the read of undefined mem in nth_back, but still a bug.

Comment thread src/types/list.rs Outdated
davidhewitt and others added 2 commits June 2, 2026 08:40
Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
@davidhewitt davidhewitt enabled auto-merge June 2, 2026 07:41
@davidhewitt davidhewitt added this pull request to the merge queue Jun 2, 2026
@davidhewitt davidhewitt removed this pull request from the merge queue due to a manual request Jun 2, 2026
@davidhewitt

Copy link
Copy Markdown
Member Author

Codex spotted a regression from #6075 in BorrowedTupleIterator::next_back; I've pushed a fix here (6a40771), will push some extra test coverage then resume merge.

@davidhewitt davidhewitt enabled auto-merge June 2, 2026 10:22
@davidhewitt davidhewitt added this pull request to the merge queue Jun 2, 2026
Merged via the queue into PyO3:main with commit 3d48a54 Jun 2, 2026
48 checks passed
@davidhewitt davidhewitt deleted the nth-ops branch June 2, 2026 15:04
mikbry added a commit to mikbry/ui that referenced this pull request Jun 12, 2026
) (#115)

Two RUSTSEC advisories published 2026-06-11 against pyo3 0.28.x, both fixed
in 0.29.0:

- RUSTSEC-2026-0176 — out-of-bounds read (memory exposure) in
  BoundListIterator/BoundTupleIterator nth/nth_back (PyO3/pyo3#6086)
- RUSTSEC-2026-0177 — missing Sync bound on PyCFunction::new_closure
  closures (PyO3/pyo3#6096)

mkui-py calls none of the affected APIs, so this is a pure dependency bump
with no source changes. Version lives in [workspace.dependencies]; mkui-py
consumes it via { workspace = true }.

Verified locally:
- cargo deny check advisories — ok (both IDs resolved)
- cargo audit — no vulnerabilities
- cargo test -p mkui-py --no-default-features --features parity-test,console — pass
- cargo fmt/clippy/test/doc/release — all green
- no new transitive deps, no MSRV change (0.29 needs 1.63; workspace is 1.87)

Closes #114

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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