Harden QueryInterface implementation (#2660)

crates/libs/bindgen/src/rust/delegates.rs

@@ -105,6 +105,10 @@ fn gen_win_delegate(writer: &Writer, def: TypeDef) -> TokenStream {
             unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
                 let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
 
+                if iid.is_null() || interface.is_null() {
+                    return ::windows_core::HRESULT(-2147467261); // E_POINTER
+                }
+
                 *interface = if *iid == <#ident as ::windows_core::ComInterface>::IID ||
                     *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID ||
                     *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID {

crates/libs/core/src/imp/weak_ref_count.rs

@@ -126,6 +126,10 @@ impl TearOff {
     unsafe extern "system" fn StrongQueryInterface(ptr: *mut std::ffi::c_void, iid: *const crate::GUID, interface: *mut *mut std::ffi::c_void) -> crate::HRESULT {
         let this = Self::from_strong_ptr(ptr);
 
+        if iid.is_null() || interface.is_null() {
+            return ::windows_core::HRESULT(-2147467261); // E_POINTER
+        }
+
         // Only directly respond to queries for the the tear-off's strong interface. This is
         // effectively a self-query.
         if *iid == IWeakReferenceSource::IID {
@@ -142,6 +146,10 @@ impl TearOff {
     unsafe extern "system" fn WeakQueryInterface(ptr: *mut std::ffi::c_void, iid: *const crate::GUID, interface: *mut *mut std::ffi::c_void) -> crate::HRESULT {
         let this = Self::from_weak_ptr(ptr);
 
+        if iid.is_null() || interface.is_null() {
+            return ::windows_core::HRESULT(-2147467261); // E_POINTER
+        }
+
         // While the weak vtable is packed into the same allocation as the strong vtable and
         // tear-off, it represents a distinct COM identity and thus does not share or delegate to
         // the object.

crates/libs/implement/src/lib.rs

@@ -64,16 +64,14 @@ pub fn implement(attributes: proc_macro::TokenStream, original_type: proc_macro:
                 }
             }
             impl #generics ::windows::core::AsImpl<#original_ident::#generics> for #interface_ident where #constraints {
+                // SAFETY: the offset is guranteed to be in bounds, and the implementation struct
+                // is guaranteed to live at least as long as `self`.
                 unsafe fn as_impl(&self) -> &#original_ident::#generics {
                     let this = ::windows::core::Interface::as_raw(self);
-                    // SAFETY: the offset is guranteed to be in bounds, and the implementation struct
-                    // is guaranteed to live at least as long as `self`.
-                    unsafe {
-                        // Subtract away the vtable offset plus 1, for the `identity` field, to get
-                        // to the impl struct which contains that original implementation type.
-                        let this = (this as *mut *mut ::core::ffi::c_void).sub(1 + #offset) as *mut #impl_ident::#generics;
-                        &(*this).this
-                    }
+                    // Subtract away the vtable offset plus 1, for the `identity` field, to get
+                    // to the impl struct which contains that original implementation type.
+                    let this = (this as *mut *mut ::core::ffi::c_void).sub(1 + #offset) as *mut #impl_ident::#generics;
+                    &(*this).this
                 }
             }
         }
@@ -105,27 +103,29 @@ pub fn implement(attributes: proc_macro::TokenStream, original_type: proc_macro:
                 &self.this
             }
             unsafe fn QueryInterface(&self, iid: *const ::windows::core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows::core::HRESULT {
-                unsafe {
-                    *interface = if *iid == <::windows::core::IUnknown as ::windows::core::ComInterface>::IID
-                        || *iid == <::windows::core::IInspectable as ::windows::core::ComInterface>::IID
-                        || *iid == <::windows::core::imp::IAgileObject as ::windows::core::ComInterface>::IID {
-                            &self.identity as *const _ as *mut _
-                    } #(#queries)* else {
-                        ::core::ptr::null_mut()
-                    };
-
-                    if !(*interface).is_null() {
-                        self.count.add_ref();
-                        return ::windows::core::HRESULT(0);
-                    }
+                if iid.is_null() || interface.is_null() {
+                    return ::windows::core::HRESULT(-2147467261); // E_POINTER
+                }
 
-                    *interface = self.count.query(iid, &self.identity as *const _ as *mut _);
+                *interface = if *iid == <::windows::core::IUnknown as ::windows::core::ComInterface>::IID
+                    || *iid == <::windows::core::IInspectable as ::windows::core::ComInterface>::IID
+                    || *iid == <::windows::core::imp::IAgileObject as ::windows::core::ComInterface>::IID {
+                        &self.identity as *const _ as *mut _
+                } #(#queries)* else {
+                    ::core::ptr::null_mut()
+                };
 
-                    if (*interface).is_null() {
-                        ::windows::core::HRESULT(0x8000_4002) // E_NOINTERFACE
-                    } else {
-                        ::windows::core::HRESULT(0)
-                    }
+                if !(*interface).is_null() {
+                    self.count.add_ref();
+                    return ::windows::core::HRESULT(0);
+                }
+
+                *interface = self.count.query(iid, &self.identity as *const _ as *mut _);
+
+                if (*interface).is_null() {
+                    ::windows::core::HRESULT(-2147467262) // E_NOINTERFACE
+                } else {
+                    ::windows::core::HRESULT(0)
                 }
             }
             fn AddRef(&self) -> u32 {
@@ -134,9 +134,7 @@ pub fn implement(attributes: proc_macro::TokenStream, original_type: proc_macro:
             unsafe fn Release(&self) -> u32 {
                 let remaining = self.count.release();
                 if remaining == 0 {
-                    unsafe {
-                        _ = ::std::boxed::Box::from_raw(self as *const Self as *mut Self);
-                    }
+                    _ = ::std::boxed::Box::from_raw(self as *const Self as *mut Self);
                 }
                 remaining
             }

crates/libs/windows/src/Windows/ApplicationModel/Background/mod.rs

@@ -5826,6 +5826,9 @@ impl<F: FnMut(::core::option::Option<&IBackgroundTaskInstance>, BackgroundTaskCa
     };
     unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
         let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
+        if iid.is_null() || interface.is_null() {
+            return ::windows_core::HRESULT(-2147467261);
+        }
         *interface = if *iid == <BackgroundTaskCanceledEventHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
         if (*interface).is_null() {
             ::windows_core::HRESULT(-2147467262)
@@ -5897,6 +5900,9 @@ impl<F: FnMut(::core::option::Option<&BackgroundTaskRegistration>, ::core::optio
     };
     unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
         let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
+        if iid.is_null() || interface.is_null() {
+            return ::windows_core::HRESULT(-2147467261);
+        }
         *interface = if *iid == <BackgroundTaskCompletedEventHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
         if (*interface).is_null() {
             ::windows_core::HRESULT(-2147467262)
@@ -5968,6 +5974,9 @@ impl<F: FnMut(::core::option::Option<&BackgroundTaskRegistration>, ::core::optio
     };
     unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
         let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
+        if iid.is_null() || interface.is_null() {
+            return ::windows_core::HRESULT(-2147467261);
+        }
         *interface = if *iid == <BackgroundTaskProgressEventHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
         if (*interface).is_null() {
             ::windows_core::HRESULT(-2147467262)

crates/libs/windows/src/Windows/ApplicationModel/DataTransfer/mod.rs

@@ -3304,6 +3304,9 @@ impl<F: FnMut(::core::option::Option<&DataProviderRequest>) -> ::windows_core::R
     };
     unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
         let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
+        if iid.is_null() || interface.is_null() {
+            return ::windows_core::HRESULT(-2147467261);
+        }
         *interface = if *iid == <DataProviderHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
         if (*interface).is_null() {
             ::windows_core::HRESULT(-2147467262)
@@ -3374,6 +3377,9 @@ impl<F: FnMut(::core::option::Option<&ShareProviderOperation>) -> ::windows_core
     };
     unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
         let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
+        if iid.is_null() || interface.is_null() {
+            return ::windows_core::HRESULT(-2147467261);
+        }
         *interface = if *iid == <ShareProviderHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
         if (*interface).is_null() {
             ::windows_core::HRESULT(-2147467262)

crates/libs/windows/src/Windows/ApplicationModel/Payments/mod.rs

@@ -2249,6 +2249,9 @@ impl<F: FnMut(::core::option::Option<&PaymentRequest>, ::core::option::Option<&P
     };
     unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
         let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
+        if iid.is_null() || interface.is_null() {
+            return ::windows_core::HRESULT(-2147467261);
+        }
         *interface = if *iid == <PaymentRequestChangedHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
         if (*interface).is_null() {
             ::windows_core::HRESULT(-2147467262)

crates/libs/windows/src/Windows/ApplicationModel/Store/mod.rs

@@ -1496,6 +1496,9 @@ impl<F: FnMut() -> ::windows_core::Result<()> + ::core::marker::Send + 'static>
     };
     unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
         let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
+        if iid.is_null() || interface.is_null() {
+            return ::windows_core::HRESULT(-2147467261);
+        }
         *interface = if *iid == <LicenseChangedEventHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
         if (*interface).is_null() {
             ::windows_core::HRESULT(-2147467262)

crates/libs/windows/src/Windows/Data/Text/mod.rs

@@ -1334,6 +1334,9 @@ impl<F: FnMut(::core::option::Option<&super::super::Foundation::Collections::IIt
     };
     unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
         let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
+        if iid.is_null() || interface.is_null() {
+            return ::windows_core::HRESULT(-2147467261);
+        }
         *interface = if *iid == <SelectableWordSegmentsTokenizingHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
         if (*interface).is_null() {
             ::windows_core::HRESULT(-2147467262)
@@ -1418,6 +1421,9 @@ impl<F: FnMut(::core::option::Option<&super::super::Foundation::Collections::IIt
     };
     unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
         let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
+        if iid.is_null() || interface.is_null() {
+            return ::windows_core::HRESULT(-2147467261);
+        }
         *interface = if *iid == <WordSegmentsTokenizingHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
         if (*interface).is_null() {
             ::windows_core::HRESULT(-2147467262)

crates/libs/windows/src/Windows/Devices/SmartCards/mod.rs

@@ -4530,6 +4530,9 @@ impl<F: FnMut(::core::option::Option<&SmartCardProvisioning>, ::core::option::Op
     };
     unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
         let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
+        if iid.is_null() || interface.is_null() {
+            return ::windows_core::HRESULT(-2147467261);
+        }
         *interface = if *iid == <SmartCardPinResetHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
         if (*interface).is_null() {
             ::windows_core::HRESULT(-2147467262)

crates/libs/windows/src/Windows/Devices/Sms/mod.rs

@@ -4413,6 +4413,9 @@ impl<F: FnMut(::core::option::Option<&SmsDevice>) -> ::windows_core::Result<()>
     };
     unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
         let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
+        if iid.is_null() || interface.is_null() {
+            return ::windows_core::HRESULT(-2147467261);
+        }
         *interface = if *iid == <SmsDeviceStatusChangedEventHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
         if (*interface).is_null() {
             ::windows_core::HRESULT(-2147467262)
@@ -4497,6 +4500,9 @@ impl<F: FnMut(::core::option::Option<&SmsDevice>, ::core::option::Option<&SmsMes
     };
     unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
         let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
+        if iid.is_null() || interface.is_null() {
+            return ::windows_core::HRESULT(-2147467261);
+        }
         *interface = if *iid == <SmsMessageReceivedEventHandler as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
         if (*interface).is_null() {
             ::windows_core::HRESULT(-2147467262)

crates/libs/windows/src/Windows/Foundation/Collections/mod.rs

@@ -1488,6 +1488,9 @@ impl<K: ::windows_core::RuntimeType + 'static, V: ::windows_core::RuntimeType +
     };
     unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
         let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
+        if iid.is_null() || interface.is_null() {
+            return ::windows_core::HRESULT(-2147467261);
+        }
         *interface = if *iid == <MapChangedEventHandler<K, V> as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
         if (*interface).is_null() {
             ::windows_core::HRESULT(-2147467262)
@@ -1571,6 +1574,9 @@ impl<T: ::windows_core::RuntimeType + 'static, F: FnMut(::core::option::Option<&
     };
     unsafe extern "system" fn QueryInterface(this: *mut ::core::ffi::c_void, iid: *const ::windows_core::GUID, interface: *mut *mut ::core::ffi::c_void) -> ::windows_core::HRESULT {
         let this = this as *mut *mut ::core::ffi::c_void as *mut Self;
+        if iid.is_null() || interface.is_null() {
+            return ::windows_core::HRESULT(-2147467261);
+        }
         *interface = if *iid == <VectorChangedEventHandler<T> as ::windows_core::ComInterface>::IID || *iid == <::windows_core::IUnknown as ::windows_core::ComInterface>::IID || *iid == <::windows_core::imp::IAgileObject as ::windows_core::ComInterface>::IID { &mut (*this).vtable as *mut _ as _ } else { ::core::ptr::null_mut() };
         if (*interface).is_null() {
             ::windows_core::HRESULT(-2147467262)