Maybe unsound in set_capability_pointer

[lwz23]

hello, thank you for your contribution in this project, I am scanning the unsoundness problem in rust project.
I notice the following code:

pub fn set_capability_pointer(
        _arena: &mut dyn BuilderArena,
        _segment_id: u32,
        mut cap_table: CapTableBuilder,
        reff: *mut WirePointer,
        cap: alloc::boxed::Box<dyn ClientHook>,
    ) {
        // TODO if ref is not null, zero object.
        unsafe {
            (*reff).set_cap(cap_table.inject_cap(cap) as u32);
        }
    }

Considering pub mod private and this is a pub function, I assume user can directly call to this function, if it's this case , I think there may exist a unsound problem in this code, eg. maybe reff is null? It will lead to UB. I suggest mark this function as unsafe or add additional check to varify the pointer. I chose to report this issue for security reasons, but don't mind if the function is not intended for external use and should be marked as pub(crate), or if this is an error report and there is actually no unsound problem.

[lwz23]

maybe same problem in

pub fn get_root(arena: &'a mut dyn BuilderArena, segment_id: u32, location: *mut u8) -> Self {
        PointerBuilder {
            arena,
            cap_table: Default::default(),
            segment_id,
            pointer: location as *mut _,
        }
    }

in this function user may pass a null pointer to crate a PointerBuilder. And then if below functions are called:

pub fn is_null(&self) -> bool {
        unsafe { (*self.pointer).is_null() }
    }

    pub fn get_struct(
        self,
        size: StructSize,
        default: Option<&'a [crate::Word]>,
    ) -> Result<StructBuilder<'a>> {
        unsafe {
            wire_helpers::get_writable_struct_pointer(
                self.arena,
                self.pointer,
                self.segment_id,
                self.cap_table,
                size,
                default,
            )
        }
    }

for example if is_null is called on a null_pointer to do unsafe { (*self.pointer).is_null() } will lead to UB.

[dwrensha]

Thanks. Everything under the private module is not intended to be directly called by downstream users. However, we can't make all of it actually private because code generated by capnpc-rust needs to be able to call some of the functions.

set_capability_pointer() does looks like something that could be made private (or perhaps pub (crate)).