From 32f2c92e90b400ae96b95ebd6a2951fa5f46a883 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 8 Jul 2026 16:16:15 -0400 Subject: [PATCH] gpui_windows: Validate window handle non-null instead of assuming The HWND received through WM_NCCREATE was stored unchecked and later rebuilt as a NonZeroIsize with new_unchecked in the raw-window-handle conversion, which would be undefined behavior if the handle were ever null. Validate the handle once where it enters our code, routing a null handle through the existing structured window-creation failure path, and store it as a NonZeroIsize so the conversion is infallible. --- crates/gpui_windows/src/window.rs | 78 +++++++++++++++++-------------- 1 file changed, 42 insertions(+), 36 deletions(-) diff --git a/crates/gpui_windows/src/window.rs b/crates/gpui_windows/src/window.rs index 5a4129b493c..b9715d4824d 100644 --- a/crates/gpui_windows/src/window.rs +++ b/crates/gpui_windows/src/window.rs @@ -87,7 +87,10 @@ pub struct WindowsWindowState { } pub(crate) struct WindowsWindowInner { - hwnd: HWND, + /// The window handle, validated non-null when the window was created and + /// stored in checked form so the `raw-window-handle` conversion can be + /// infallible. Use [`Self::hwnd`] to get it as an `HWND`. + non_zero_hwnd: NonZeroIsize, drop_target_helper: IDropTargetHelper, pub(crate) state: WindowsWindowState, system_settings: WindowsSystemSettings, @@ -244,6 +247,8 @@ impl WindowsWindowState { impl WindowsWindowInner { fn new(context: &mut WindowCreateContext, hwnd: HWND, cs: &CREATESTRUCTW) -> Result> { + let non_zero_hwnd = NonZeroIsize::new(hwnd.0 as isize) + .context("WM_NCCREATE received a null window handle")?; let state = WindowsWindowState::new( hwnd, &context.directx_devices, @@ -258,7 +263,7 @@ impl WindowsWindowInner { )?; Ok(Rc::new(Self { - hwnd, + non_zero_hwnd, drop_target_helper: context.drop_target_helper.clone(), state, handle: context.handle, @@ -275,6 +280,10 @@ impl WindowsWindowInner { })) } + fn hwnd(&self) -> HWND { + HWND(self.non_zero_hwnd.get() as _) + } + fn toggle_fullscreen(self: &Rc) { let this = self.clone(); self.executor @@ -292,9 +301,9 @@ impl WindowsWindowInner { this.state.fullscreen_restore_bounds.set(window_bounds); let style = - WINDOW_STYLE(unsafe { get_window_long(this.hwnd, GWL_STYLE) } as _); + WINDOW_STYLE(unsafe { get_window_long(this.hwnd(), GWL_STYLE) } as _); let mut rc = RECT::default(); - unsafe { GetWindowRect(this.hwnd, &mut rc) } + unsafe { GetWindowRect(this.hwnd(), &mut rc) } .context("failed to get window rect") .log_err(); let _ = this.state.fullscreen.set(Some(StyleAndBounds { @@ -320,11 +329,11 @@ impl WindowsWindowInner { } } }; - set_non_rude_hwnd(this.hwnd, !this.state.is_fullscreen()); - unsafe { set_window_long(this.hwnd, GWL_STYLE, style.0 as isize) }; + set_non_rude_hwnd(this.hwnd(), !this.state.is_fullscreen()); + unsafe { set_window_long(this.hwnd(), GWL_STYLE, style.0 as isize) }; unsafe { SetWindowPos( - this.hwnd, + this.hwnd(), None, x, y, @@ -344,19 +353,19 @@ impl WindowsWindowInner { }; match open_status.state { WindowOpenState::Maximized => unsafe { - SetWindowPlacement(self.hwnd, &open_status.placement) + SetWindowPlacement(self.hwnd(), &open_status.placement) .context("failed to set window placement")?; - ShowWindowAsync(self.hwnd, SW_MAXIMIZE).ok()?; + ShowWindowAsync(self.hwnd(), SW_MAXIMIZE).ok()?; }, WindowOpenState::Fullscreen => { unsafe { - SetWindowPlacement(self.hwnd, &open_status.placement) + SetWindowPlacement(self.hwnd(), &open_status.placement) .context("failed to set window placement")? }; self.toggle_fullscreen(); } WindowOpenState::Windowed => unsafe { - SetWindowPlacement(self.hwnd, &open_status.placement) + SetWindowPlacement(self.hwnd(), &open_status.placement) .context("failed to set window placement")?; }, } @@ -564,10 +573,7 @@ impl WindowsWindow { impl rwh::HasWindowHandle for WindowsWindow { fn window_handle(&self) -> std::result::Result, rwh::HandleError> { - let raw = rwh::Win32WindowHandle::new(unsafe { - NonZeroIsize::new_unchecked(self.0.hwnd.0 as isize) - }) - .into(); + let raw = rwh::Win32WindowHandle::new(self.0.non_zero_hwnd).into(); Ok(unsafe { rwh::WindowHandle::borrow_raw(raw) }) } } @@ -585,7 +591,7 @@ impl Drop for WindowsWindow { self.0 .executor .spawn(async move { - let handle = this.hwnd; + let handle = this.hwnd(); unsafe { RevokeDragDrop(handle).log_err(); DestroyWindow(handle).log_err(); @@ -617,7 +623,7 @@ impl PlatformWindow for WindowsWindow { } fn resize(&mut self, size: Size) { - let hwnd = self.0.hwnd; + let hwnd = self.0.hwnd(); let bounds = gpui::bounds(self.bounds().origin, size).to_device_pixels(self.scale_factor()); let rect = calculate_window_rect(bounds, &self.state.border_offset); @@ -660,7 +666,7 @@ impl PlatformWindow for WindowsWindow { GetCursorPos(&mut point) .context("unable to get cursor position") .log_err(); - ScreenToClient(self.0.hwnd, &mut point).ok().log_err(); + ScreenToClient(self.0.hwnd(), &mut point).ok().log_err(); point }; logical_point(point.x as f32, point.y as f32, scale_factor) @@ -692,7 +698,7 @@ impl PlatformWindow for WindowsWindow { let (done_tx, done_rx) = oneshot::channel(); let msg = msg.to_string(); let detail_string = detail.map(|detail| detail.to_string()); - let handle = self.0.hwnd; + let handle = self.0.hwnd(); let answers = answers.to_vec(); self.0 .executor @@ -767,7 +773,7 @@ impl PlatformWindow for WindowsWindow { } fn activate(&self) { - let hwnd = self.0.hwnd; + let hwnd = self.0.hwnd(); let this = self.0.clone(); self.0 .executor @@ -821,7 +827,7 @@ impl PlatformWindow for WindowsWindow { } fn is_active(&self) -> bool { - self.0.hwnd == unsafe { GetActiveWindow() } + self.0.hwnd() == unsafe { GetActiveWindow() } } fn is_hovered(&self) -> bool { @@ -837,14 +843,14 @@ impl PlatformWindow for WindowsWindow { } fn set_title(&mut self, title: &str) { - unsafe { SetWindowTextW(self.0.hwnd, &HSTRING::from(title)) } + unsafe { SetWindowTextW(self.0.hwnd(), &HSTRING::from(title)) } .inspect_err(|e| log::error!("Set title failed: {e}")) .ok(); } fn set_background_appearance(&self, background_appearance: WindowBackgroundAppearance) { self.state.background_appearance.set(background_appearance); - let hwnd = self.0.hwnd; + let hwnd = self.0.hwnd(); // using Dwm APIs for Mica and MicaAlt backdrops. // others follow the set_window_composition_attribute approach @@ -870,13 +876,13 @@ impl PlatformWindow for WindowsWindow { } fn minimize(&self) { - unsafe { ShowWindowAsync(self.0.hwnd, SW_MINIMIZE).ok().log_err() }; + unsafe { ShowWindowAsync(self.0.hwnd(), SW_MINIMIZE).ok().log_err() }; } fn zoom(&self) { unsafe { - if IsWindowVisible(self.0.hwnd).as_bool() { - ShowWindowAsync(self.0.hwnd, SW_MAXIMIZE).ok().log_err(); + if IsWindowVisible(self.0.hwnd()).as_bool() { + ShowWindowAsync(self.0.hwnd(), SW_MAXIMIZE).ok().log_err(); } else if let Some(mut status) = self.state.initial_placement.take() { status.state = WindowOpenState::Maximized; self.state.initial_placement.set(Some(status)); @@ -885,7 +891,7 @@ impl PlatformWindow for WindowsWindow { } fn toggle_fullscreen(&self) { - if unsafe { IsWindowVisible(self.0.hwnd).as_bool() } { + if unsafe { IsWindowVisible(self.0.hwnd()).as_bool() } { self.0.toggle_fullscreen(); } else if let Some(mut status) = self.state.initial_placement.take() { status.state = WindowOpenState::Fullscreen; @@ -966,7 +972,7 @@ impl PlatformWindow for WindowsWindow { } fn get_raw_handle(&self) -> HWND { - self.0.hwnd + self.0.hwnd() } fn gpu_specs(&self) -> Option { @@ -981,7 +987,7 @@ impl PlatformWindow for WindowsWindow { + ((bounds.size.height.as_f32() * scale_factor) as i32 / 2), }; - self.0.update_ime_position(self.0.hwnd, caret_position); + self.0.update_ime_position(self.0.hwnd(), caret_position); } fn play_system_bell(&self) { @@ -991,10 +997,10 @@ impl PlatformWindow for WindowsWindow { fn a11y_init(&self, callbacks: gpui::A11yCallbacks) { let action_handler = A11yActionHandler(callbacks.action); - let is_focused = unsafe { GetForegroundWindow() } == self.0.hwnd; + let is_focused = unsafe { GetForegroundWindow() } == self.0.hwnd(); let adapter = accesskit_windows::Adapter::new( - accesskit_windows::HWND(self.0.hwnd.0), + accesskit_windows::HWND(self.0.hwnd().0), is_focused, action_handler, ); @@ -1101,7 +1107,7 @@ impl IDropTarget_Impl for WindowsDragDropHandler_Impl { }); ReleaseStgMedium(&mut idata); let mut cursor_position = cursor_position; - ScreenToClient(self.0.hwnd, &mut cursor_position) + ScreenToClient(self.0.hwnd(), &mut cursor_position) .ok() .log_err(); let scale_factor = self.0.state.scale_factor.get(); @@ -1119,7 +1125,7 @@ impl IDropTarget_Impl for WindowsDragDropHandler_Impl { } self.0 .drop_target_helper - .DragEnter(self.0.hwnd, idata_obj, &cursor_position, *pdweffect) + .DragEnter(self.0.hwnd(), idata_obj, &cursor_position, *pdweffect) .log_err(); } Ok(()) @@ -1138,7 +1144,7 @@ impl IDropTarget_Impl for WindowsDragDropHandler_Impl { .drop_target_helper .DragOver(&cursor_position, *pdweffect) .log_err(); - ScreenToClient(self.0.hwnd, &mut cursor_position) + ScreenToClient(self.0.hwnd(), &mut cursor_position) .ok() .log_err(); } @@ -1180,7 +1186,7 @@ impl IDropTarget_Impl for WindowsDragDropHandler_Impl { .drop_target_helper .Drop(idata_obj, &cursor_position, *pdweffect) .log_err(); - ScreenToClient(self.0.hwnd, &mut cursor_position) + ScreenToClient(self.0.hwnd(), &mut cursor_position) .ok() .log_err(); } @@ -1423,7 +1429,7 @@ fn get_module_handle() -> HMODULE { } fn register_drag_drop(window: &Rc) -> Result<()> { - let window_handle = window.hwnd; + let window_handle = window.hwnd(); let handler = WindowsDragDropHandler(window.clone()); // The lifetime of `IDropTarget` is handled by Windows, it won't release until // we call `RevokeDragDrop`.