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

Add recursive call limit #1305

Open
hackaugusto opened this issue Apr 3, 2024 · 4 comments
Open

Add recursive call limit #1305

hackaugusto opened this issue Apr 3, 2024 · 4 comments
Labels
processor Related to Miden VM processor

Comments

@hackaugusto
Copy link
Contributor

hackaugusto commented Apr 3, 2024

Currently the VM uses recursion to interpret procedure calls, which means an attacker can easily write a program that will exhaust the host stack, killing it.

POC: A dyncall instruction in MASM causes the Rust program to recurse. The following program can be used to create infinite recursion:

proc.rec
  dynexec
end

begin
  procref.rec
  exec.rec
end

Running the above causes a crash:

thread 'main' has overflowed its stack
fatal runtime error: stack overflow
[1]    99618 abort      cargo run --features="executable" -- run --assembly t.masm
@hackaugusto
Copy link
Contributor Author

Another approach is to change the way the bytecode is interpreted, and rely exclusively on the max cycle counter to prevent running out of main memory. This is related to #1217

@hackaugusto hackaugusto added good first issue Good for newcomers processor Related to Miden VM processor labels Apr 3, 2024
@cyberbono3
Copy link

Can I start working on it?

@hackaugusto
Copy link
Contributor Author

Can I start working on it?

@bobbinth @bitwalker ping. I'm not sure what are the plans with the interpreter rewrite to remove the recursion calls.

@bobbinth
Copy link
Contributor

@cyberbono3 - thank you for wanting to help with this! I would probably hold off on this issue for now as we will be refactoring how the processor runs though the program in the next couple of weeks (the Program struct will be moved to table-based description of MAST - see #1226). So, the mechanism that may apply to the current implementation may need to be different after the refactor.

@plafer plafer removed the good first issue Good for newcomers label Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
processor Related to Miden VM processor
Projects
None yet
Development

No branches or pull requests

4 participants