Unsoundness in scaly

[xizheyin]

I discovered an unsoundness in scaly, but I couldn't locate its repository, so I'm submitting it here. Implementation of Index::index did not check the bound.

  82 | impl<T: Copy> Index<usize> for Array<T> {
  83 |     type Output = T;
  84 |     fn index(&self, offset: usize) -> &Self::Output {
  85 |         unsafe { &*self.vector.data.offset(offset as isize) }
  86 |     }
  87 | }

reproduce case:

use scaly::Array;

fn main() {
    let mut a: Array<u8> = Array::new();
    a.add(1);

    // Safe API OOB: Index<usize> does unchecked pointer offset
    // This should be UB (Miri will flag).
    let v = a[1];
    std::hint::black_box(v);
}

miri reports:

error: Undefined Behavior: pointer not dereferenceable: pointer must be dereferenceable for 16 bytes, but got 0x20000[noalloc] which is a dangling pointer (it has no provenance)
  --> /home/test/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/scaly-0.0.37/src/containers/array.rs:58:38
   |
58 |                 let exclusive_page = (*_own_page).allocate_exclusive_page();
   |                                      ^^^^^^^^^^^^ Undefined Behavior occurred here
   |
...

[xizheyin]

append_character can trigger UB.

use scaly::String;
use scaly::memory::{Heap, Page, StackBucket};

fn main() {
    let mut heap = Heap::create();
    let root_stack_bucket = StackBucket::create(&mut heap);
    let root_page = Page::get(root_stack_bucket as usize);

    // Call append_character on an empty string, which will use (len - 1) to calculate the write position internally.
    // For an empty string with len=0, it will underflow, causing an OOB write.
    let s = String::new(root_page, "");
    let out = s.append_character(root_page, 'a');
    std::hint::black_box(out);
}

run miri:

RUSTFLAGS="-C overflow-checks=off" \
MIRIFLAGS="-Zmiri-disable-stacked-borrows -Zmiri-ignore-leaks" \
cargo miri run

outputs(OOB):

error: Undefined Behavior: accessing memory based on pointer with alignment 1, but alignment 4 is required
   --> /home/test/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/scaly-0.0.37/src/containers/string.rs:123:13
    |
123 |             write(char_pointer, c);
    |             ^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
    |
...

[xizheyin]

get_length trigger UB as well.

use scaly::String;

fn main() {
    // Construct an illegal encoding: only 0x80, indicating that the length needs to continue reading the next byte.
    // get_length will read OOB（safe API）。
    let buf = vec![0x80u8];
    let s = String { data: buf.as_ptr() };
    std::mem::forget(buf);

    let len = s.get_length();
    std::hint::black_box(len);
}

miri outputs:

error: Undefined Behavior: memory access failed: attempting to access 1 byte, but got alloc179+0x1 which is at or beyond the end of the allocation of size 1 byte
   --> /home/test/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/scaly-0.0.37/src/containers/string.rs:137:37
    |
137 |             let byte: u8 = unsafe { *(self.data.offset(index)) };
    |                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
    |

[xizheyin]

to_c_string trigger UB. A string of length 130 triggers an out-of-bounds write at index+1+length.

write(dest.offset((index + 1 + length) as isize), zero);, while dest has length of 1+length.

use scaly::String;
use scaly::memory::{Heap, Page, StackBucket};

fn main() {
    let mut heap = Heap::create();
    let root_stack_bucket = StackBucket::create(&mut heap);
    let root_page = Page::get(root_stack_bucket as usize);

    // 长度 >= 128 会导致长度编码占用 2 字节，
    // to_c_string 在写结尾 NUL 时使用 (index+1+length)，发生越界写。
    let long = "A".repeat(130);
    let s = String::new(root_page, &long);
    let c = s.to_c_string(root_page);
    std::hint::black_box(c);
}

[djc]

Note that this crate was last published 6 years ago, has no dependent crates and no documentation.

[xizheyin]

🤔 Is this no longer maintained? It seems many APIs have similar issues. It can still be downloaded from crates.io. It has been downloaded 137,314 times.

[djc]

🤔 Is this no longer maintained?

That seems likely with no releases for 6 years.

It has been downloaded 137,314 times.

That's all-time downloads. It's been downloaded 315 times over the last 90 days.

[xizheyin]

This brings a question to mind. How does Rust handle deprecated and unmaintained repositories? When these repositories enter the ecosystem, legacy bugs could cause contamination.

[djc]

We have unmaintained advisories. Anyway, we also usually try to get approval from a maintainer before publishing an advisory. Suggest you mention the maintainer(s) on this thread.

[xizheyin]

oops. I just found the original repository.

[xtqqczze]

The Rust code in this project is only kept for historical reasons. It is no longer maintained.

Originally posted by @rschleitzer in https://github.com/rschleitzer/Scaly/issues/5#issuecomment-3794515367

[djc]

Happy to review an informational = "unsound" advisory for this crate.