Cloning ArrowArray causes segfault on drop.

[zirconium-n]

Following code easily segfault. Ffi_ArrowArray's Clone or Drop is incorrect, or did I break contract of export_array_to_c?

use std::sync::Arc;

use arrow2::array::UInt32Array;
use arrow2::ffi::{Ffi_ArrowArray, export_array_to_c};

fn main() {
    let arr = Arc::new(UInt32Array::from_vec(vec![1, 2, 3, 4]));
    let mut arr_ffi = Ffi_ArrowArray::empty();

    unsafe {
        export_array_to_c(arr.clone(), &mut arr_ffi as *mut _);
    };

    let _ = arr_ffi.clone();

    // Segfault on drop
}

[jorgecarleitao]

Thank you very much for this report 🙇

I agree that this is a (serious) issue. My understanding is that we must not #derive Clone, since there is no guarantees over the pointers' lifetimes when the derived clone is used together with our implementation of Drop.

If you agree with this assessment, I suggest the following course of action:

1. Create a fix (remove the derive(Clone)) and backport it for:

   • 0.7
   • 0.8
   • 0.9
   • latest main

2. create an entry in the rust advisory so that we can communicate the fix and mitigations (i.e. do not clone FFi_ArrowArray).

Let me know what do you think.

[zirconium-n]

Yes, I agree removing #[derive(Clone)] is the best solution.

Another way would be removing impl Drop and provide a unsafe fn drop_array, but it's probably too hard to use it correctly.

P.S. ArrowSchema and ArrowArrayStream have the same problem too.

[jorgecarleitao]

I have now fixed this in main, v0.7, v0.8 and v0.9. I has created an advisory, RUSTSEC-2022-0012 via rustsec/advisory-db#1204

Thank you again for this report that positively contributes to the soundness of this crate in particular and rust's ecosystem more broadly.