fish segfaults on nested `eval`

[fuzzypixelz]

I'm running fish v3.5.1, on openSUSE Linux 6.0 and TERM=xterm-256color; I have no customizations.

I was trying to do something akin to bash's search and replace in the last command:

> echo Hello
Hello
> eval $(string replace Hello Hi $history[1])
Hi
> eval $(string replace Hello Hi $history[1])
[1]    13363 segmentation fault (core dumped)  fish

If you do this from a parent shell then you get a SIGSEGV (Address boundary error). I really don't have a good idea for what might be causing this, but I suspect it's because there might be an infinite recursion in eval above (not sure about the semantics of fish).

Still, having one's shell segfault is much worse than catching this error or even arbitrarily limiting the depth.

[mqudsi]

Yeah, this is a classic stack overflow. Nice find.

It'd be easy enough to catch a stack overflow in the naïve case (eval leads to identical eval) but there are always going to be variations on that possible that wouldn't be caught by such a straightforward check.

I don't think there is any real security risk here since you're unlikely to be interacting with a fish instance that's running at a higher privilege than you have the rights to (plenty of more obvious ways to get what you want in that case). Obviously something like a pure fish http server listening on the public internet would be at risk, but I think you probably have bigger problems in that case!

I think just limiting either eval or overall execution depth to some sane value would be fine. On my Linux machine, presumably with the default 8MB stack size, the overflow occurred after 28785 stack frames and the stack depth for a single eval call is around 10 frames, meaning this was at around 2900 eval recursions. The eval loop runs on the main thread; Linux assigns background threads the same default stack size) but other supported operating systems like FreeBSD and macOS limit the background stack size to much smaller values (2 MiB and 512 KiB, respectively).

I think either 250 or 500 might be a conservative but still fair recursion limit. We can restrict it to just eval loops, but this can still happen with regular recursion (e.g. depth-first directory search in fish) so it might be better to apply it for all parser executions.

EDIT

We already have a function call stack limit of 128, but it's possible to bypass it if you avoid making function calls, as you've seen.

[floam]

Seems like only zsh and ksh93 of what I've tried has a hardcoded recursion limit, and lets the user override with FUNCNEST. Bash will segfault.

[floam]

Both zsh and ksh93 use 1000.