Skip to content

fix: avoid use-after-free when re-entering ostream_redirect - #6205

Merged
rwgk merged 1 commit into
pybind:masterfrom
sayed0200:iostream-redirect-reentry-uaf
Oct 19, 2026
Merged

rwgk merged 1 commit into
pybind:masterfrom
sayed0200:iostream-redirect-reentry-uaf

Conversation

@sayed0200

Copy link
Copy Markdown
Contributor

Description

detail::OstreamRedirect::enter() runs redirect_stdout.reset(new scoped_ostream_redirect()). Calling __enter__ a second time without an __exit__ in between (nesting the same ostream_redirect context manager) constructs the replacement before reset() destroys the current one, so the new scoped_ostream_redirect records std::cout.rdbuf() while that still points at the buffer owned by the redirect reset() is about to delete. After the delete the live redirect holds a dangling old streambuf, and on __exit__ its destructor restores std::cout/std::cerr to that freed buffer. The next write to the stream dereferences freed memory.

Minimal repro, stdout only:

r = m.ostream_redirect(stdout=True, stderr=False)
r.__enter__()
r.__enter__()
r.__exit__()
m.raw_output("boom")  # use-after-free, segfaults

Releasing the existing redirect before constructing the replacement makes the new one capture the real original buffer, so no dangling pointer is left behind. Single enter/exit and sequential reuse of the same object are unaffected.

The added test_redirect_reentry segfaults on the current headers and passes with the fix.

This fix was found and prepared with AI assistance and reviewed by me before submitting.

Suggested changelog entry:

  • Fixed a use-after-free in py::add_ostream_redirect when the context manager's __enter__ was entered again without an intervening __exit__.

@rwgk rwgk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

@rwgk
rwgk merged commit fdec8c3 into pybind:master Oct 19, 2026
84 checks passed
@github-actions github-actions Bot added the needs changelog Possibly needs a changelog entry label Oct 19, 2026
@rwgk

rwgk commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator

For completeness / JIC someone wants to pick this up later:

While reviewing this PR, codex GPT-6.1-Sol ultra suggested:

PR #6205 prevents the dangling-buffer failure, but does not make the manager fully nesting-aware: an inner exit still ends redirection. The added test exercises stdout with double entry and one exit; stderr and actual nested-with behaviour deserve consideration. Ordinary nesting with separate manager instances is not the reported problem.

Full nesting support would be a small, self-contained follow-up. I would make repeated entry into one manager behave like using a fresh manager at each nesting level.

Currently, OstreamRedirect owns one stdout/stderr guard pair. It would instead own a stack of guard pairs:

  • Each __enter__ creates and pushes a new pair, preserving the existing guards.
  • Each __exit__ destroys only the top pair, restoring the preceding level’s buffers.
  • The final exit restores the original C++ buffers.

That would give this the expected behavior:

r = m.ostream_redirect()
with r:
    write_cpp("outer-before")
    with r:
        write_cpp("inner")
    write_cpp("outer-after")  # Still redirected

A depth counter would suffice for keeping the first redirect active until the final exit. A stack gives more complete semantics: each entry captures the current sys.stdout/sys.stderr, including when Python changes those streams between nesting levels, and it composes with other managers between entries.

There are three implementation details to handle carefully:

  1. Stable guard addresses. Existing guards contain pointers to preceding buffers. Heap-allocated frames, held through unique_ptr, would keep those buffers stationary when the stack grows.
  2. Reverse-order destruction. Explicitly pop frames from the top, including when destroying a manager with outstanding entries.
  3. Entry failure rollback. Construct each frame locally and push it only after both requested redirects succeed. A failed stderr setup must restore stdout and preserve the previous nesting level.

Tests should exercise actual nested with blocks for stdout, stderr, and both; changing Python destinations between entries; exception unwinding; buffered output; and sequential reuse.

I’d expect tens of implementation lines, with tests forming most of the follow-up. Keeping #6205 focused on preventing the dangling-buffer failure seems reasonable; nesting support establishes additional behavior that deserves its own tests and documentation.

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

Labels

needs changelog Possibly needs a changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants