Skip to content
New issue

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

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

Already on GitHub? # to your account

Stream callback procedure is not unwind safe #20

Closed
Phosphorus15 opened this issue Sep 13, 2019 · 2 comments
Closed

Stream callback procedure is not unwind safe #20

Phosphorus15 opened this issue Sep 13, 2019 · 2 comments

Comments

@Phosphorus15
Copy link

It is observed that the stream_callback and stream_finished_callback functions are not unwind safe, as their definitions shown below.

let mut stream_data: Box<StreamUserData<I, O>> = unsafe { mem::transmute(user_data) };
...
let result = match stream_data.callback
    {
        Some(ref mut f) => (*f)(input_buffer, output_buffer, timeinfo, flags),
        None => StreamCallbackResult::Abort,
    };

    mem::forget(stream_data);

If the user-provided closure could possibly panic, the mem::forget of boxed StreamUserData would not be reachable, which causes its memory to be deallocated, thus resulting in an use after free.

Since the StreamUserData contains two function pointers which might be executed later-on, it is obvious that an arbitrary code execution can be constructed maliciously by this way. Therefore, this is highly-vulnerable and should be fixed.

BartMassey added a commit to BartMassey-upstream/portaudio-rs that referenced this issue May 13, 2020
Two uses of mem::forget() after user callbacks were replaced
with Box::leak() before the user callbacks to ensure that
unowned memory was not improperly freed on panic.
BartMassey added a commit to BartMassey-upstream/portaudio-rs that referenced this issue May 13, 2020
Two uses of mem::forget() after user callbacks were replaced
with Box::leak() before the user callbacks to ensure that
unowned memory was not improperly freed on panic.
@mvdnes
Copy link
Owner

mvdnes commented May 14, 2020

Resolved in #21

@mvdnes mvdnes closed this as completed May 14, 2020
@mvdnes
Copy link
Owner

mvdnes commented May 14, 2020

Thank you for the report! 👍

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants