Skip to content

fix: avoid dangling reference in optional/variant of reference_wrapper - #6232

Open
sayed0200 wants to merge 1 commit into
pybind:masterfrom
sayed0200:stl-optional-variant-refwrapper-uaf
Open

sayed0200 wants to merge 1 commit into
pybind:masterfrom
sayed0200:stl-optional-variant-refwrapper-uaf

Conversation

@sayed0200

Copy link
Copy Markdown
Contributor

Description

Closes #5637.

std::optional<std::reference_wrapper<T>> segfaults when used as a function argument, even though the plain std::reference_wrapper<T> and std::optional<T> forms both work. optional_caster::load() built its inner value caster as a frame-local, emplaced the loaded value into the optional, then let that local go out of scope when the function returned. For a reference_wrapper the loaded value is a reference into the inner caster's own storage, which is exactly where the T lives whenever it has to be materialized from Python (an Eigen::MatrixXd, a std::string, an arithmetic value); once the inner caster was destroyed that reference dangled, so the first read inside the bound call was a heap-use-after-free. I reproduced it under AddressSanitizer and found the same shape in variant_caster, whose per-alternative casters were also frame-locals. The fix keeps the inner caster(s) alive for as long as the composite caster lives, which is the duration of the bound call: optional_caster now holds its inner caster as a member, and variant_caster holds a std::tuple of its alternative casters and indexes into it. The new tests round-trip a long, heap-allocated std::string through std::optional<std::reference_wrapper<std::string>> and std::variant<int, std::reference_wrapper<std::string>>; both fail on master and pass with this change, and the rest of the test_stl suite stays green.

Suggested changelog entry:

  • Fixed a dangling reference (use-after-free) when passing std::optional or std::variant of std::reference_wrapper as an argument, where the referenced value is materialized by its caster.

optional_caster and variant_caster destroyed their inner value caster when
load() returned, but std::reference_wrapper<T> stores a reference into that
caster's own storage when T is materialized from Python (e.g. std::string,
Eigen matrix). The stored reference then dangled, so the first use during the
bound call was a heap-use-after-free (pybind#5637).

Hold the inner caster(s) as members so they outlive the loaded value for the
duration of the bound call.

This branch has not been deployed

No deployments
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.

[BUG]: function with argument type std::optional<std::reference_wrapper<Eigen::...>> gives segmentation fault

1 participant