chore: audit unsafe blocks in `SqlCipherConnection::handle_ios_wal_co…

…mpat`

In this case, the fix was just to add safety notes, which was nice.

keystore/src/connection/platform/generic/mod.rs

@@ -208,34 +208,53 @@ impl SqlCipherConnection {
                 pub static kSecAttrAccessible: CFStringRef;
             }
 
+            macro_rules! wrap_under_get_rule {
+                ($external_ref:expr) => {
+                    // SAFETY: The method `CFString::wrap_under_get_rule` is required by rustc to be marked as unsafe
+                    // because accessing any static item via FFI is intrinsically unsafe. For this reason, we can't
+                    // just create a safe wrapper function here; rustc immediately flags it with an [E0133] because it
+                    // needs that FFI static item to be its parameter.
+                    //
+                    // There isn't safety documentation anywhere in the CFString or TCFType documentation, and
+                    // the implementation does only [basic checks], so we can assume that the main property we
+                    // need to uphold is "the reference is not null".
+                    //
+                    // [E0133]: https://doc.rust-lang.org/stable/error_codes/E0133.html
+                    // [basic checks]: https://github.com/servo/core-foundation-rs/blob/core-foundation-v0.10.0/core-foundation/src/lib.rs#L91-L95
+                    unsafe { CFString::wrap_under_get_rule($external_ref) }
+                };
+            }
+
             // Create a query that matches a:
             let query_params = CFDictionary::from_CFType_pairs(&[
                 // Class GenericPassword
-                (unsafe { CFString::wrap_under_get_rule(kSecClass) }, unsafe {
-                    CFString::wrap_under_get_rule(kSecClassGenericPassword).as_CFType()
-                }),
+                (
+                    wrap_under_get_rule!(kSecClass),
+                    wrap_under_get_rule!(kSecClassGenericPassword).as_CFType(),
+                ),
                 // with Service = "wire.com"
                 (
-                    unsafe { CFString::wrap_under_get_rule(kSecAttrService) },
+                    wrap_under_get_rule!(kSecAttrService),
                     CFString::from(WIRE_SERVICE_NAME).as_CFType(),
                 ),
                 // Holding account name = `key` (in the following form: `keystore_salt_[sha256(file_path)]`)
-                (
-                    unsafe { CFString::wrap_under_get_rule(kSecAttrAccount) },
-                    CFString::from(key).as_CFType(),
-                ),
+                (wrap_under_get_rule!(kSecAttrAccount), CFString::from(key).as_CFType()),
             ]);
 
             // And now we ask to update the following properties:
             let payload_params = CFDictionary::from_CFType_pairs(&[(
                 // Keychain Accessibility setting
                 // See: https://developer.apple.com/documentation/security/ksecattraccessible
-                unsafe { CFString::wrap_under_get_rule(kSecAttrAccessible) },
+                wrap_under_get_rule!(kSecAttrAccessible),
                 // Set to AccessibleAfterFirstUnlock (i.e. is accessible after the first post-boot unlock)
-                unsafe { CFString::wrap_under_get_rule(kSecAttrAccessibleAfterFirstUnlock).as_CFType() },
+                wrap_under_get_rule!(kSecAttrAccessibleAfterFirstUnlock).as_CFType(),
             )]);
 
             // Update the item in the keychain
+            //
+            // SAFETY: As before, the main source of unsafety here appears to simply be that this is an FFI function
+            // and nobody has constructed a safe wrapper aroud it. And sure, we can't trust the safety properties that
+            // external code has. But on the other hand, we can't not call this function. So the unsafe block should be fine.
             match unsafe {
                 security_framework_sys::keychain_item::SecItemUpdate(
                     query_params.as_concrete_TypeRef(),