search

harfbuzz / rustybuzz

A complete harfbuzz's shaping algorithm port to Rust
MIT License
551 stars 37 forks source link

Unsound issue of `set_len` #135

Closed CXWorks closed 3 weeks ago

CXWorks commented 3 weeks ago

Hi, thanks for your time to read this issue. Our static analysis tool found there might be an unsound issue in your set_len implementation of buffer:

https://github.com/RazrFalcon/rustybuzz/blob/71e33a06bc941338b116c558fb3f09d48818237c/src/hb/buffer.rs#L1213-L1216

Because this can change the logical length of the buffer and in the ensure method, if the length exceeds the max length, altough it returns false indicating the failure, the len of buffer is still changed. As a reference, in std library, all the set_len method are marked as unsafe:

https://github.com/rust-lang/rust/blob/63a0bdd5622eaf6b9524702f055bb4525acfc9f2/library/alloc/src/vec/mod.rs#L1849-L1853

Thanks again for your time.

RazrFalcon commented 3 weeks ago

There is nothing "unsound" about it. Our Buffer doesn't operate on raw pointers. It's just a minor logic error that cannot effect memory safety.

Also, it's nearly impossible to make ensure fail, so it's fine anyways.

CXWorks commented 3 weeks ago

Oh, I sould double check the raw pointer, thanks for your time!