From f7370be3a37b186902893a7c0bbf9a0b59543d0c Mon Sep 17 00:00:00 2001 From: Gautier Minster Date: Tue, 23 Jun 2020 15:21:15 -0700 Subject: [PATCH 1/2] audio: fix device name use-after-free in AudioDevice::open Fixes a use-after-free in `AudioDevice::open`, which occurs when selecting a particular device by name (as opposed to using a default device). Specifically, when extracting a C-style string pointer from a `Option`, the option was consumed, its contents turned into a pointer, and the original `CString` dropped. After that the C-style string pointer is dangling, as its backing rust CString has been freed. To fix this, the CString must be kept alive, which is achieved by simply creating an intermediary `Option<&CString>`, and consuming that. --- src/sdl2/audio.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/sdl2/audio.rs b/src/sdl2/audio.rs index 60f668d2ab7..0551e186489 100644 --- a/src/sdl2/audio.rs +++ b/src/sdl2/audio.rs @@ -652,7 +652,10 @@ impl AudioDevice { Some(device) => Some(CString::new(device).unwrap()), None => None }; - let device_ptr = device.map_or(ptr::null(), |s| s.as_ptr()); + // Warning: map_or consumes its argument; `device.map_or()` would therefore consume the + // CString and drop it, making device_ptr a dangling pointer! To avoid that we downgrade + // device to an Option<&_> first. + let device_ptr = device.as_ref().map_or(ptr::null(), |s| s.as_ptr()); let iscapture_flag = if capture { 1 } else { 0 }; let device_id = sys::SDL_OpenAudioDevice( From 482a039f92cf713792fd11e123dbd6622dd4a921 Mon Sep 17 00:00:00 2001 From: Gautier Minster Date: Tue, 23 Jun 2020 17:01:11 -0700 Subject: [PATCH 2/2] FIXUP: also fix AudioQueue::open_queue --- src/sdl2/audio.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/sdl2/audio.rs b/src/sdl2/audio.rs index 0551e186489..6e77b4f115e 100644 --- a/src/sdl2/audio.rs +++ b/src/sdl2/audio.rs @@ -559,7 +559,10 @@ impl<'a, Channel: AudioFormatNum> AudioQueue { Some(device) => Some(CString::new(device).unwrap()), None => None }; - let device_ptr = device.map_or(ptr::null(), |s| s.as_ptr()); + // Warning: map_or consumes its argument; `device.map_or()` would therefore consume the + // CString and drop it, making device_ptr a dangling pointer! To avoid that we downgrade + // device to an Option<&_> first. + let device_ptr = device.as_ref().map_or(ptr::null(), |s| s.as_ptr()); let iscapture_flag = 0; let device_id = sys::SDL_OpenAudioDevice(