Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Segfault with Memory::grow/MemoryView interaction #3516

Closed
Imberflur opened this issue Jan 21, 2023 · 2 comments
Closed

Segfault with Memory::grow/MemoryView interaction #3516

Imberflur opened this issue Jan 21, 2023 · 2 comments
Assignees
Labels
bug Something isn't working priority-high High priority issue
Milestone

Comments

@Imberflur
Copy link

Describe the bug

https://docs.rs/wasmer/3.1.0/wasmer/struct.MemoryView.html documents:

After a memory is grown a view must not be used anymore.

However, it is possible to do this in safe code and instead of producing an error or panicking this results in a segfault. So there is a soundness issue with the interaction of Memory::grow and Memory::view.

wasmer v3.1.0 | rustc 1.66.1 (90743e729 2023-01-10) | x86_64

Steps to reproduce

use wasmer::{Memory, MemoryType, Store};

fn main() {
    // The default configuration gives memory plenty of room to grow in-place.
    let tunables = wasmer::BaseTunables {
        static_memory_bound: 4.into(),
        static_memory_offset_guard_size: 1024 * 64 * 4,
        dynamic_memory_offset_guard_size: 1024 * 64 * 4,
    };
    let engine = wasmer::EngineBuilder::headless().engine();

    let mut store = Store::new_with_tunables(engine, tunables);

    let memory = Memory::new(&mut store, MemoryType::new(10, None, false)).unwrap();
    let view = memory.view(&store);
    memory.grow(&mut store, 1024).unwrap();
    view.write_u8(0, 4u8).unwrap();
}
> cargo r
fish: Job 1, 'cargo r' terminated by signal SIGSEGV (Address boundary error)

Expected behavior

One of the following:

  • Producing an error, panicking, or aborting.
  • An API that statically prevents this, e.g. by having the Store borrowed for the lifetime of MemoryView.
  • An unsafe API.

Actual behavior

Access to freed memory in safe code. Segfault.

@ptitSeb ptitSeb added bug Something isn't working priority-medium Medium priority issue labels Jan 24, 2023
@ptitSeb ptitSeb added this to the v3.2 milestone Jan 24, 2023
@ptitSeb ptitSeb added priority-high High priority issue and removed priority-medium Medium priority issue labels Jan 24, 2023
@ptitSeb ptitSeb self-assigned this Jan 24, 2023
@Michael-F-Bryan
Copy link
Contributor

@ptitSeb was this resolved in #3580?

@ptitSeb
Copy link
Contributor

ptitSeb commented Apr 12, 2023

Yes it was

@ptitSeb ptitSeb closed this as completed Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority-high High priority issue
Projects
None yet
Development

No branches or pull requests

3 participants