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

vm: fix illegal memory access not being detected #83

Merged
merged 2 commits into from
Dec 15, 2024

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented Dec 12, 2024

Summary

Fix VM memory access checks not rejecting memory access when the
accessed memory region crosses the uint64 upper limit, allowing for
read/write access to outside the guest memory.

Details

The test in checkmem did not account for unsigned integer wrap
around. If x is greater than or equal to the start of the virtual
memory and x + len wraps around, then no error was reported if the
wrapped-around value was less than the virtual memory's upper limit.

Unsigned integer wrap around is now explicitly tested for, leading to
checkmem returning an error if detected.


Notes For Reviewers

  • a bug discovered with skully

Summary
=======

Fix VM memory access checks not rejecting memory access when the
accessed memory region crosses the `uint64` upper limit, allowing for
read/write access to outside the guest memory.

Details
=======

The test in `checkmem` did not account for unsigned integer wrap
around. If `x` is greater than or equal to the start of the virtual
memory and `x + len` wraps around, then no error was reported if the
wrapped-around value was less than the virtual memory's upper limit.

Unsigned integer wrap around is now explicitly tested for, leading to
`checkmem` returning an error if detected.
@zerbina zerbina added the bug Something isn't working label Dec 12, 2024
@zerbina zerbina merged commit fde1eb1 into nim-works:main Dec 15, 2024
5 checks passed
@zerbina zerbina deleted the vm-fix-access-check branch December 15, 2024 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant