refactor: enable safer unsafe clippy lints (#551)

> Checks for the result of a `&self`-taking `as_ptr` being cast to a
mutable pointer.


https://rust-lang.github.io/rust-clippy/stable/index.html#as_ptr_cast_mut

> Checks for casts of a function pointer to any integer type.


https://rust-lang.github.io/rust-clippy/stable/index.html#fn_to_numeric_cast_any

> Checks for `as` casts between raw pointers that change their
constness, namely `*const T` to `*mut T` and `*mut T` to `*const T`.


https://rust-lang.github.io/rust-clippy/stable/index.html#ptr_cast_constness

> Checks for `// SAFETY:` comments on safe code.


https://rust-lang.github.io/rust-clippy/stable/index.html#unnecessary_safety_comment

> Checks for the doc comments of publicly visible unsafe functions and
warns if there is no `# Safety` section.


https://rust-lang.github.io/rust-clippy/stable/index.html#missing_safety_doc

Author: TheBestTvarynka

Cargo.toml

@@ -46,6 +46,15 @@ redundant_lifetimes = "warn"
 trivial_numeric_casts = "warn"
 unused_qualifications = "warn"
 
+[workspace.lints.clippy]
+
+# == Safer unsafe == #
+as_ptr_cast_mut = "warn"
+fn_to_numeric_cast_any = "warn"
+ptr_cast_constness = "warn"
+unnecessary_safety_comment = "warn"
+missing_safety_doc = "warn"
+
 [workspace.dependencies]
 uuid = { version = "1.18", default-features = false }
 tracing = { version = "0.1",  default-features = false }

ffi/src/sspi/common.rs

@@ -105,14 +105,16 @@ pub unsafe extern "system" fn AcceptSecurityContext(
                 None => return ErrorKind::InvalidHandle.to_u32().unwrap(),
             };
 
-        // SAFETY:
-        // - `ph_context` is either null or convertible to a reference.
-        // - The values behind `ph_context.dw_lower` and `ph_context.dw_upper` pointers are allocated by an SSPI function.
-        let mut sspi_context_ptr = try_execute!(unsafe { p_ctxt_handle_to_sspi_context(
-            &mut ph_context,
-            Some(security_package_name),
-            attributes,
-        )});
+        let mut sspi_context_ptr = try_execute!(
+            // SAFETY:
+            // - `ph_context` is either null or convertible to a reference.
+            // - The values behind `ph_context.dw_lower` and `ph_context.dw_upper` pointers are allocated by an SSPI function.
+            unsafe { p_ctxt_handle_to_sspi_context(
+                &mut ph_context,
+                Some(security_package_name),
+                attributes,
+            )}
+        );
 
         // SAFETY: `sspi_context_ptr` is a valid, local pointer to the `SspiHandle` allocated by the `p_ctx_handle_to_sspi_context`.
         let sspi_context = unsafe { sspi_context_ptr.as_mut() };
@@ -158,12 +160,14 @@ pub unsafe extern "system" fn AcceptSecurityContext(
         // - `p_output` is guaranteed to be non-null due to the prior check.
         // - `p_output` points to a valid `SecBufferDesc` structure.
         let p_buffers = unsafe { (*p_output).p_buffers };
-        // SAFETY:
-        // - `p_buffers` is a valid pointer to an array of security buffers.
-        // - The memory region `p_buffers` points to is valid for writes of `from_buffers.len()` elements.
-        // - For each element in the `p_buffers` array, the `pv_buffer` pointer points to a valid memory,
-        //   that is valid for writes of `cb_buffer` bytes.
-        try_execute!(unsafe { copy_to_c_sec_buffer(p_buffers, &output_tokens, false) });
+        try_execute!(
+            // SAFETY:
+            // - `p_buffers` is a valid pointer to an array of security buffers.
+            // - The memory region `p_buffers` points to is valid for writes of `from_buffers.len()` elements.
+            // - For each element in the `p_buffers` array, the `pv_buffer` pointer points to a valid memory,
+            //   that is valid for writes of `cb_buffer` bytes.
+            unsafe { copy_to_c_sec_buffer(p_buffers, &output_tokens, false) }
+        );
 
         // SAFETY: `ph_new_context` is convertible to a reference.
         let ph_new_context = unsafe { ph_new_context.as_mut() }.expect("ph_new_context is non-null");
@@ -219,14 +223,16 @@ pub unsafe extern "system" fn CompleteAuthToken(
         let p_buffers = unsafe { (*p_token).p_buffers };
         check_null!(p_buffers);
 
-        // SAFETY:
-        // - `ph_context` is either null or convertible to a reference.
-        // - The values behind `ph_context.dw_lower` and `ph_context.dw_upper` pointers are allocated by an SSPI function.
-        let mut sspi_context_ptr = try_execute!(unsafe { p_ctxt_handle_to_sspi_context(
-            &mut ph_context,
-            None,
-            &CredentialsAttributes::default()
-        )});
+        let mut sspi_context_ptr = try_execute!(
+            // SAFETY:
+            // - `ph_context` is either null or convertible to a reference.
+            // - The values behind `ph_context.dw_lower` and `ph_context.dw_upper` pointers are allocated by an SSPI function.
+            unsafe { p_ctxt_handle_to_sspi_context(
+                &mut ph_context,
+                None,
+                &CredentialsAttributes::default()
+            )}
+        );
 
         // SAFETY: `sspi_context_ptr` is a valid, local pointer to the `SspiHandle` allocated by the `p_ctx_handle_to_sspi_context`.
         let sspi_context = unsafe { sspi_context_ptr.as_mut() };
@@ -273,14 +279,16 @@ pub unsafe extern "system" fn DeleteSecurityContext(mut ph_context: PCtxtHandle)
     catch_panic!(
         check_null!(ph_context);
 
-        // SAFETY:
-        // - `ph_context` is convertible to a reference.
-        // - The values behind `ph_context.dw_lower` and `ph_context.dw_upper` pointers are allocated by an SSPI function.
-        let mut sspi_context_ptr = try_execute!(unsafe { p_ctxt_handle_to_sspi_context(
-            &mut ph_context,
-            None,
-            &CredentialsAttributes::default()
-        )});
+        let mut sspi_context_ptr = try_execute!(
+            // SAFETY:
+            // - `ph_context` is convertible to a reference.
+            // - The values behind `ph_context.dw_lower` and `ph_context.dw_upper` pointers are allocated by an SSPI function.
+            unsafe { p_ctxt_handle_to_sspi_context(
+                &mut ph_context,
+                None,
+                &CredentialsAttributes::default()
+            )}
+        );
 
         // SAFETY: `sspi_context_ptr` is a valid, local pointer to the `SspiHandle` allocated by the `p_ctx_handle_to_sspi_context`.
         let _context: Box<SspiHandle> = unsafe {
@@ -436,14 +444,16 @@ pub unsafe extern "system" fn EncryptMessage(
         let p_buffers = unsafe { (*p_message).p_buffers };
         check_null!(p_buffers);
 
-        // SAFETY:
-        // - `ph_context` is convertible to a reference.
-        // - The values behind `ph_context.dw_lower` and `ph_context.dw_upper` pointers are allocated by an SSPI function.
-        let mut sspi_context_ptr = try_execute!(unsafe { p_ctxt_handle_to_sspi_context(
-            &mut ph_context,
-            None,
-            &CredentialsAttributes::default()
-        )});
+        let mut sspi_context_ptr = try_execute!(
+            // SAFETY:
+            // - `ph_context` is convertible to a reference.
+            // - The values behind `ph_context.dw_lower` and `ph_context.dw_upper` pointers are allocated by an SSPI function.
+            unsafe { p_ctxt_handle_to_sspi_context(
+                &mut ph_context,
+                None,
+                &CredentialsAttributes::default()
+            )}
+        );
 
         // SAFETY: `sspi_context_ptr` is a valid, local pointer to the `SspiHandle` allocated by the `p_ctx_handle_to_sspi_context`.
         let sspi_context = unsafe { sspi_context_ptr.as_mut() };
@@ -460,24 +470,28 @@ pub unsafe extern "system" fn EncryptMessage(
             from_raw_parts(p_buffers, len)
         };
 
-        // SAFETY:
-        // - `raw_buffers` array contains valid `SecBuffer` structures.
-        // - Each `SecBuffer` have a valid `pv_buffer` pointer that is valid for reads of `cb_buffer` bytes.
-        let mut message = try_execute!(unsafe { p_sec_buffers_to_decrypt_buffers(raw_buffers)});
+        let mut message = try_execute!(
+            // SAFETY:
+            // - `raw_buffers` array contains valid `SecBuffer` structures.
+            // - Each `SecBuffer` have a valid `pv_buffer` pointer that is valid for reads of `cb_buffer` bytes.
+            unsafe { p_sec_buffers_to_decrypt_buffers(raw_buffers) }
+        );
 
         let result_status = sspi_context.encrypt_message(
             EncryptionFlags::from_bits(f_qop.try_into().unwrap()).unwrap(),
             &mut message,
             message_seq_no.try_into().unwrap(),
         );
 
-        // SAFETY:
-        // - `p_buffers` is a valid pointer to an array of security buffers.
-        // - The memory region `p_buffers` points to is valid for writes of `from_buffers.len()` elements.
-        // - For each element in the `p_buffers` array, the `pv_buffer` pointer points to a valid memory,
-        //   that is valid for writes of `cb_buffer` bytes.
-        // - There is enough space in the buffers, since we are writing encrypted data back to the same buffers it was read.
-        try_execute!(unsafe { copy_decrypted_buffers(p_buffers, message) });
+        try_execute!(
+            // SAFETY:
+            // - `p_buffers` is a valid pointer to an array of security buffers.
+            // - The memory region `p_buffers` points to is valid for writes of `from_buffers.len()` elements.
+            // - For each element in the `p_buffers` array, the `pv_buffer` pointer points to a valid memory,
+            //   that is valid for writes of `cb_buffer` bytes.
+            // - There is enough space in the buffers, since we are writing encrypted data back to the same buffers it was read.
+            unsafe { copy_decrypted_buffers(p_buffers, message) }
+        );
 
         let result = try_execute!(result_status);
         result.to_u32().unwrap()
@@ -518,14 +532,16 @@ pub unsafe extern "system" fn DecryptMessage(
         let p_buffers = unsafe { (*p_message).p_buffers };
         check_null!(p_buffers);
 
-        // SAFETY:
-        // - `ph_context` is convertible to a reference.
-        // - The values behind `ph_context.dw_lower` and `ph_context.dw_upper` pointers are allocated by an SSPI function.
-        let mut sspi_context_ptr = try_execute!(unsafe { p_ctxt_handle_to_sspi_context(
-            &mut ph_context,
-            None,
-            &CredentialsAttributes::default()
-        )});
+        let mut sspi_context_ptr = try_execute!(
+            // SAFETY:
+            // - `ph_context` is convertible to a reference.
+            // - The values behind `ph_context.dw_lower` and `ph_context.dw_upper` pointers are allocated by an SSPI function.
+            unsafe { p_ctxt_handle_to_sspi_context(
+                &mut ph_context,
+                None,
+                &CredentialsAttributes::default()
+            )}
+        );
 
         // SAFETY: `sspi_context_ptr` is a valid, local pointer to the `SspiHandle` allocated by the `p_ctx_handle_to_sspi_context`.
         let sspi_context = unsafe { sspi_context_ptr.as_mut() };
@@ -540,24 +556,28 @@ pub unsafe extern "system" fn DecryptMessage(
         // - The memory region `p_buffers` points to is valid for reads of `len` elements.
         let raw_buffers = unsafe { from_raw_parts(p_buffers, len) };
 
-        // SAFETY:
-        // - `raw_buffers` array contains valid `SecBuffer` structures.
-        // - Each `SecBuffer` have a valid `pv_buffer` pointer that is valid for reads of `cb_buffer` bytes.
-        let mut message = try_execute!(unsafe { p_sec_buffers_to_decrypt_buffers(raw_buffers) });
+        let mut message = try_execute!(
+            // SAFETY:
+            // - `raw_buffers` array contains valid `SecBuffer` structures.
+            // - Each `SecBuffer` have a valid `pv_buffer` pointer that is valid for reads of `cb_buffer` bytes.
+            unsafe { p_sec_buffers_to_decrypt_buffers(raw_buffers) }
+        );
 
         let (decryption_flags, result_status) =
             match sspi_context.decrypt_message(&mut message, message_seq_no.try_into().unwrap()) {
                 Ok(flags) => (flags, Ok(())),
                 Err(error) => (DecryptionFlags::empty(), Err(error)),
             };
 
-        // SAFETY:
-        // - `p_buffers` is a valid pointer to an array of security buffers.
-        // - The memory region `p_buffers` points to is valid for writes of `from_buffers.len()` elements.
-        // - For each element in the `p_buffers` array, the `pv_buffer` pointer points to a valid memory,
-        //   that is valid for writes of `cb_buffer` bytes.
-        // - There is enough space in the buffers, since we are writing decrypted data back to the same buffers it was read.
-        try_execute!(unsafe { copy_decrypted_buffers(p_buffers, message) });
+        try_execute!(
+            // SAFETY:
+            // - `p_buffers` is a valid pointer to an array of security buffers.
+            // - The memory region `p_buffers` points to is valid for writes of `from_buffers.len()` elements.
+            // - For each element in the `p_buffers` array, the `pv_buffer` pointer points to a valid memory,
+            //   that is valid for writes of `cb_buffer` bytes.
+            // - There is enough space in the buffers, since we are writing decrypted data back to the same buffers it was read.
+            unsafe { copy_decrypted_buffers(p_buffers, message) }
+        );
 
         // `pf_qop` can be null if this library is used as a CredSsp security package
         if !pf_qop.is_null() {
@@ -612,7 +632,7 @@ unsafe fn p_sec_buffers_to_decrypt_buffers(raw_buffers: &[SecBuffer]) -> sspi::R
 
 /// Copies Rust-security-buffers into C-security-buffers.
 ///
-/// # Safety:
+/// # Safety
 ///
 /// This function accepts owned [from_buffers] to avoid UB and other errors. Rust-buffers should
 /// not be used after the data is copied into C-buffers.

ffi/src/sspi/credentials_attributes.rs

@@ -66,7 +66,7 @@ pub struct SecPkgCredentialsKdcProxySettingsW {
 
 /// Extracts [KdcProxySettings].
 ///
-/// # Safety:
+/// # Safety
 ///
 /// * The pointer value must be [SecPkgCredentialsKdcProxySettingsW].
 ///   The fields `proxy_server_offset`, `proxy_server_length`, `client_tls_cred_offset` and `client_tls_cred_length` must contain valid values.

ffi/src/sspi/sec_buffer.rs

@@ -23,7 +23,7 @@ pub struct SecBufferDesc {
 
 pub type PSecBufferDesc = *mut SecBufferDesc;
 
-/// # Safety:
+/// # Safety
 ///
 /// * The input pointer can be null.
 /// * If the input pointer is non-null, then it must point to the valid [SecBufferDesc] structure. Moreover,
@@ -77,7 +77,7 @@ pub(crate) unsafe fn p_sec_buffers_to_security_buffers(raw_buffers: &[SecBuffer]
 
 /// Copies buffers from `from_buffers` to `to_buffers`.
 ///
-/// # Safety:
+/// # Safety
 ///
 /// The `to_buffers` must be a valid pointer to an array of security buffers. It must be valid for writes of `from_buffers.len()` elements.
 /// Additionally, if `allocate` is `false`, each `to_buffers[i].pv_buffer` must be a valid pointer for writes of `from_buffers[i].buffer.len()` elements.