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

gnovm: decide maximum recursion depth perhaps defined by gas or with a preset maximum #3549

Open
odeke-em opened this issue Jan 19, 2025 · 1 comment
Labels
security Security-sensitive issue

Comments

@odeke-em
Copy link
Contributor

I'd expect a smart contracting VM to have certain pre-defined limits that would control deep call routines and also memory limits because when I try to find the 60th Fibonacci number, this program hangs and most likely runs out of memory https://play.gno.land/p/FOEYk0mBinL

package hello

func main() {
	print(fib(60))
}

func fib(n int) int {
  if n <= 1 {
    return 1
  }
  return fib(n-1) + fib(n-2)
}

I think one way to reduce security blast radiuses is to particularly look at OpCall and for each function call graph, determine when it has run out of the allowed maximum recursion depth.

Solidity has a max recursion depth of 1024 per https://docs.soliditylang.org/en/latest/introduction-to-smart-contracts.html#message-calls

@kristovatlas
Copy link
Contributor

Unless I'm missing something, the gas system does cause this to terminate.

package hello

func Main() int {
	return fib(60)
}

func fib(n int) int {
	if n <= 1 {
		return 1
	}
	return fib(n-1) + fib(n-2)
}
% gnokey maketx call --pkgpath "gno.land/r/kristovatlas/gno3549b" --func "Main" --gas-fee 10000000ugnot --gas-wanted 800000 --broadcast --remote localhost:26657 Dev
TX HASH:
--= Error =--
Data: allocation limit exceeded
Msg Traces:
    0  redacted/gno/tm2/pkg/errors/errors.go:28 - deliver transaction failed: log:msg:0,success:false,log:--= Error =--
Data: &errors.errorString{s:"allocation limit exceeded"}
Msg Traces:
    0  redacted/gno/gno.land/pkg/sdk/vm/keeper.go:513 - VM call panic: allocation limit exceeded
...snip...

@kristovatlas kristovatlas added the security Security-sensitive issue label Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security Security-sensitive issue
Projects
Status: Triage
Development

No branches or pull requests

2 participants