capnproto / capnproto-rust

Cap'n Proto for Rust
MIT License
2.06k stars 222 forks source link

Dynamic parsing of fields may panic #498

Closed crane-may closed 6 months ago

crane-may commented 6 months ago

When I use the has_named interface some_struct.has_named("some_field_name"), sometimes it's panic.

thread 'main' panicked at XXXX\capnp-0.19.2\src\schema.rs:76:31:
index out of bounds: the len is 4 but the index is 4

Then I check the code: https://github.com/capnproto/capnproto-rust/blob/master/capnp/src/schema.rs

pub fn find_field_by_name(&self, name: &str) -> Result<Option<Field>> {
        let fields = self.get_fields()?;
        let mut lower: usize = 0;
        let mut upper: usize = self.raw.generic.members_by_name.len();
        let mut mid: usize = (lower + upper) / 2;
        let mut candidate_index = self.raw.generic.members_by_name[mid];
        let mut candidate_name = fields.get(candidate_index).get_proto().get_name()?;

        while lower < upper {
            use core::cmp::Ordering;
            match (&name).partial_cmp(&candidate_name) {
                Some(Ordering::Equal) => return Ok(Some(fields.get(candidate_index))),
                Some(Ordering::Greater) => lower = mid + 1,
                Some(Ordering::Less) => upper = mid,
                None => unreachable!(),
            }
            mid = (lower + upper) / 2;
            candidate_index = self.raw.generic.members_by_name[mid];
            candidate_name = fields.get(candidate_index).get_proto().get_name()?;
        }
        Ok(None)
    }

I'm guessing that mid might be equal to upper, which would cause the array to go out of bounds. I don't know if my findings are correct.

dwrensha commented 6 months ago

Thanks for the report! This should be fixed in https://github.com/capnproto/capnproto-rust/pull/499.

Note that for a field that does not show up in the schema, has_named() will return an error FieldNotFound rather than false. This matches the upstream behavior in capnproto-c++: https://github.com/capnproto/capnproto/blob/5d83271d808467af8f4dbaa4c73daaf53a90b5ec/c%2B%2B/src/capnp/dynamic.c%2B%2B#L1004-L1006 https://github.com/capnproto/capnproto/blob/5d83271d808467af8f4dbaa4c73daaf53a90b5ec/c%2B%2B/src/capnp/schema.c%2B%2B#L507-L513