V OpenVet
cargo / android_system_properties / audit
cargo : android_system_properties @ 0.1.5
PE Patrick Elsen signed 2026-05-27 published 2026-05-27

Claims

has-binarieshas-build-exechas-fuzz-testshas-install-exechas-integration-testshas-property-testshas-unit-testsimpl-algorithmimpl-concurrencyimpl-cryptoimpl-datastructureimpl-interpreterimpl-jitimpl-parserimpl-protocolis-benignunsafe-documentedunsafe-minimalunsafe-safeunsafe-testeduses-concurrencyuses-cryptouses-environmentuses-execuses-filesystemuses-interpreteruses-jituses-networkuses-unsafe

Summary

android_system_properties 0.1.5 is a minimal Android system-properties wrapper that uses dlopen(RTLD_NOLOAD) + dlsym to discover property entry points at runtime, supporting both pre- and post-Android-L APIs. Pure FFI shim, no I/O. The ~dozen unsafe operations were reviewed and are sound, but two low-severity quality issues are filed: a panic-on-non-UTF-8 in the callback path (FINDING-1), and a complete absence of // SAFETY: comments (FINDING-2). is-benign.

Report

Subject

android_system_properties is a minimal Android system-properties wrapper that exposes one type, AndroidSystemProperties, with a get(name) -> Option<String> method. Unlike alternatives that link statically against Android's libc, this crate uses dlopen(RTLD_NOLOAD) + dlsym at runtime to discover the property-system entry points, so a single Rust binary can run against both pre- and post-Android-L (5.0) devices: the newer callback-based API (__system_property_find + __system_property_read_callback) is preferred when present, and the legacy __system_property_get is used as a fallback. On non-Android targets the crate compiles to a no-op stub.

Methodology

The published crate contents were compared against the upstream Git repository at the commit recorded in .cargo_vcs_info.json using diff -r contents/src vcs/src, which showed no differences. The single source file (src/lib.rs, 221 LoC) was read in full. The example (examples/time_zone.rs) and the Cargo.toml were reviewed. Each unsafe site was inspected for the invariants it relies on and the documentation those invariants carry; the index arithmetic in the legacy property-get path was traced manually.

Tools: openvet 0.6.0 for workspace and audit data management; diff (Apple) for the byte-level comparison; git (2.51) for the upstream checkout; grep/ripgrep for capability surveys.

Results

The published .crate matches the VCS tree byte-for-byte across src/; Cargo.toml differences are cargo's standard normalisation. The crate ships no binary assets (justifying has-binaries), has no build.rs, no proc-macro library, and no install hooks (justifying has-build-exec and has-install-exec). It performs no filesystem operations of its own — dlopen reads a shared library from the dynamic loader's cache but the crate does no path manipulation or file I/O (justifying uses-filesystem) — and no network, environment, JIT, interpreter, concurrency, or cryptographic operations (justifying uses-network, uses-environment, uses-jit, uses-interpreter, uses-concurrency, uses-crypto). The crate implements none of impl-crypto, impl-parser, impl-interpreter, impl-jit, impl-protocol, impl-datastructure, impl-algorithm, or impl-concurrency — it is a thin wrapper around three Android libc functions. dlopen does load executable code, but it is not the kind of subprocess/dynamic-evaluation usage the audit taxonomy means by uses-exec (which is reserved for exec()/subprocess patterns).

The crate is essentially an FFI wrapper, and accordingly it does use unsafe (justifying uses-unsafe). Approximately a dozen unsafe operations exist: the dlopen/dlsym/dlclose libc calls, four mem::transmute casts from *const c_void to specific extern "C" fn signatures, two property-system FFI calls ((find_fn)(...), (read_callback_fn)(...), (get_fn)(...)), a buffer.set_len() after the legacy property API writes into a pre-sized Vec, the extern "C" fn C-callback used by the newer API, and unsafe impl Send + Sync for the wrapper struct.

Soundness of the unsafe was reviewed and is upheld:

  • dlopen(b"libc.so\0", RTLD_NOLOAD): RTLD_NOLOAD makes this query-only — it returns the existing handle if libc.so is already loaded (which it always is on Android), and null otherwise. The null case is handled.
  • dlsym results are cast via mem::transmute to extern "C" fn pointers matching the Android documented signatures. This pattern is standard for libc-dlsym; the signatures are correct against Android's system_properties.h.
  • The legacy __system_property_get path allocates a Vec::with_capacity(PROPERTY_VALUE_MAX) where PROPERTY_VALUE_MAX = 92 matches Android's PROP_VALUE_MAX, passes a raw pointer, then assert!(len as usize <= buffer.capacity()) before set_len(len). Correctness depends on Android's documented contract that the function never writes more than PROP_VALUE_MAX bytes; the assert is defensive but cannot prevent UB if the C side has already overflowed.
  • unsafe impl Send + Sync: AndroidSystemProperties is a passive holder of a libc handle and three function pointers. After construction it is read-only; the underlying Android libc property store is documented thread-safe. Sound.
  • Drop calls dlclose. Because the function pointers live on the same struct as the handle, no outstanding aliases exist when the handle is closed.

Two low-severity quality findings (FINDING-1 and FINDING-2) were recorded:

  • FINDING-1 captures a real-world panic risk: the C callback at src/lib.rs:47-50 calls cvalue.to_str().unwrap(), which panics across an FFI boundary on non-UTF-8 property values. The legacy path correctly uses String::from_utf8(buffer).ok(); the callback path should do the equivalent.
  • FINDING-2 records that none of the unsafe blocks (FFI calls, mem::transmutes, set_len, the C callback signature, the Send/Sync impls) carry // SAFETY: comments. The invariants are knowable to a careful reader but should be documented. This is the basis for declining unsafe-documented.

The crate justifies unsafe-safe (every block was reviewed and the invariants hold) and unsafe-minimal (unsafe is used only where strictly necessary — FFI). It declines unsafe-tested: there is no test suite of any kind. No #[test] modules in src/ (justifying has-unit-tests), no tests/ directory (justifying has-integration-tests), no fuzz harness (justifying has-fuzz-tests), no proptest (justifying has-property-tests). The crate is presumably exercised in the wild by wgpu and other downstream consumers (the README notes its scope is limited to "what's needed by wgpu"), but no in-tree tests exist.

No malicious behaviour was observed (justifying is-benign).

Conclusion

android_system_properties is a small, focused FFI wrapper for Android's property system, using a sensible dlopen/dlsym pattern to avoid hard-linking against a libc version. The implementation is correct under careful review, but two quality issues stand out: a panic risk in the modern-API callback path (FINDING-1) and a lack of // SAFETY: documentation on the dozen-or-so unsafe operations (FINDING-2). Neither is a security or correctness defect for typical Android system properties, which are ASCII; both should be addressed for code-review-ability and robustness on edge-case property values. Suitable for use as-is by callers that only need standard, well-formed properties.

Findings(2)

FINDING-1 quality low

property_callback panics on invalid UTF-8

In src/lib.rs:47-50, the C callback used with __system_property_read_callback calls cvalue.to_str().unwrap().to_string() on the C-string value pointer. If an Android system property contains non-UTF-8 bytes (uncommon but not specified to be impossible by the Android docs), this panics across an FFI boundary — undefined behaviour for C-side callers expecting a normal callback return. A safer pattern would be to use to_str().ok().map(|s| s.to_string()) and propagate None via the payload, or to use from_utf8_lossy(). The same legacy-API path at line 203 correctly uses String::from_utf8(buffer).ok() and returns None on invalid UTF-8.

FINDING-2 quality low

Unsafe blocks lack safety comments

The crate contains roughly a dozen unsafe operations (FFI calls, mem::transmute of dlsym results to function pointers, buffer.set_len, the C callback signature, and the unsafe impl Send/Sync for AndroidSystemProperties). None of these carry a // SAFETY: comment. The invariants they rely on (libc.so already loaded due to RTLD_NOLOAD; Android's __system_property_get writes at most PROPERTY_VALUE_MAX = 92 bytes; function pointers obtained from dlsym match the declared signatures; the type contains only handles and extern "C" fns and is therefore safe to share across threads) are knowable to a careful reader but should be written down. This is the basis for declining unsafe-documented.

Annotations(1)

src/lib.rs

src/lib.rs, line 47-50

unsafe fn property_callback(payload: *mut String, _name: *const c_char, value: *const c_char, _serial: u32) {
    let cvalue = CStr::from_ptr(value);
    (*payload) = cvalue.to_str().unwrap().to_string();
}

C-callback writes a String into a caller-provided payload pointer. The to_str().unwrap() panics on invalid UTF-8 across an FFI boundary; see FINDING-1.

src/lib.rs, line 79-80

unsafe impl Send for AndroidSystemProperties {}
unsafe impl Sync for AndroidSystemProperties {}

unsafe impl Send for AndroidSystemProperties / unsafe impl Sync for .... The type contains a *mut c_void (dlopen handle) and three Option<extern "C" fn> slots. After new() the struct is immutable and reads from libc's property store, which is thread-safe on Android. Send/Sync is sound; the missing safety comment is part of FINDING-2.

src/lib.rs, line 94-136

    #[cfg(target_os = "android")]
    /// Create an entry point for accessing Android properties.
    pub fn new() -> Self {
        let libc_so = unsafe { libc::dlopen(b"libc.so\0".as_ptr().cast(), libc::RTLD_NOLOAD) };

        let mut properties = AndroidSystemProperties {
            libc_so,
            find_fn: None,
            read_callback_fn: None,
            get_fn: None,
        };

        if libc_so.is_null() {
            return properties;
        }


        unsafe fn load_fn(libc_so: *mut c_void, name: &[u8]) -> Option<*const c_void> {
            let fn_ptr = libc::dlsym(libc_so, name.as_ptr().cast());

            if fn_ptr.is_null() {
                return None;
            }

            Some(fn_ptr)
        }

        unsafe {
            properties.read_callback_fn = load_fn(libc_so, b"__system_property_read_callback\0")
                .map(|raw| mem::transmute::<*const c_void, SystemPropertyReadCallbackFn>(raw));

            properties.find_fn = load_fn(libc_so, b"__system_property_find\0")
                .map(|raw| mem::transmute::<*const c_void, SystemPropertyFindFn>(raw));

            // Fallback for old versions of Android.
            if properties.read_callback_fn.is_none() || properties.find_fn.is_none() {
                properties.get_fn = load_fn(libc_so, b"__system_property_get\0")
                    .map(|raw| mem::transmute::<*const c_void, SystemPropertyGetFn>(raw));
            }
        }

        properties
    }

new() on Android: dlopen("libc.so", RTLD_NOLOAD) — RTLD_NOLOAD returns a handle only if libc.so is already loaded (which it always is in any Android process), avoiding the cost and complications of a fresh load. Then dlsym for the three property functions; if the new callback-based pair (__system_property_read_callback + __system_property_find) is missing, falls back to the legacy __system_property_get. The function pointer slots are Option<extern "C" fn> so a missing symbol just leaves None.

src/lib.rs, line 173-210

    pub fn get_from_cstr(&self, cname: &std::ffi::CStr) -> Option<String> {
        // If available, use the recommended approach to accessing properties (Android L and onward).
        if let (Some(find_fn), Some(read_callback_fn)) = (self.find_fn, self.read_callback_fn) {
            let info = unsafe { (find_fn)(cname.as_ptr()) };

            if info.is_null() {
                return None;
            }

            let mut result = String::new();

            unsafe {
                (read_callback_fn)(info, property_callback, &mut result);
            }

            return Some(result);
        }

        // Fall back to the older approach.
        if let Some(get_fn) = self.get_fn {
            // The constant is PROP_VALUE_MAX in Android's libc/include/sys/system_properties.h
            const PROPERTY_VALUE_MAX: usize = 92;
            let mut buffer: Vec<u8> = Vec::with_capacity(PROPERTY_VALUE_MAX);
            let raw = buffer.as_mut_ptr() as *mut c_char;

            let len = unsafe { (get_fn)(cname.as_ptr(), raw) };

            if len > 0 {
                assert!(len as usize <= buffer.capacity());
                unsafe { buffer.set_len(len as usize); }
                String::from_utf8(buffer).ok()
            } else {
                None
            }
        } else {
            None
        }
    }

get_from_cstr: prefers the modern callback-based API. The legacy path (lines 192-209) uses Vec::with_capacity(PROPERTY_VALUE_MAX) (92 bytes, per Android's libc/include/sys/system_properties.h), hands a raw pointer to the C function, then assert!(len as usize <= buffer.capacity()) before set_len. The assert is defensive — if the C side overflowed the buffer, UB would already have happened. Correctness depends on Android's documented contract that __system_property_get writes at most PROP_VALUE_MAX bytes. String::from_utf8(buffer).ok() returns None on invalid UTF-8 in this path, unlike the callback path (FINDING-1).

src/lib.rs, line 213-221

impl Drop for AndroidSystemProperties {
    fn drop(&mut self) {
        if !self.libc_so.is_null() {
            unsafe {
                libc::dlclose(self.libc_so);
            }
        }
    }
}

Drop calls dlclose. Since the function pointers are stored on the same struct as the handle, they cannot outlive the dlclose. Sound.