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

Buffer overflow due to incorrect stack size calculation in mrb_funcall_with_block #3421

Closed
clayton-shopify opened this Issue Feb 1, 2017 · 1 comment

Comments

Projects
None yet
2 participants
@clayton-shopify
Contributor

clayton-shopify commented Feb 1, 2017

The following input demonstrates a crash:

Hash.new { |h,k| h[k] }[0]

This code should exhaust the stack due to infinite recursion, but MRuby crashes in mrb_funcall_with_block after a small number of recursion steps. At this line, an attempt is made to write beyond the end of the stack:

mruby/src/vm.c

Line 424 in 6420951

mrb->c->stack[argc+1] = blk;

I suspect the stack size calculation here is incorrect:

mruby/src/vm.c

Lines 408 to 409 in 6420951

ci->nregs = p->body.irep->nregs + argc;
stack_extend(mrb, ci->nregs, argc+2);

But I don't really understand the mechanics of the stack well enough to know what the correct fix is.

This issue was reported by https://hackerone.com/ston3

@matz matz closed this in 719f700 Feb 13, 2017

@matz

This comment has been minimized.

Show comment
Hide comment
@matz

matz Feb 13, 2017

Member

719f700 fixes the buffer overflow but still causes a stack overflow because C stack grows too fast due to infinite recursion.

Member

matz commented Feb 13, 2017

719f700 fixes the buffer overflow but still causes a stack overflow because C stack grows too fast due to infinite recursion.

matz added a commit that referenced this issue Feb 13, 2017

Do not use mrb_funcall() if Hash#default is not overridden; ref #3421
This change reduces the recursion level, but does not solve the stack
overflow issue entirely.

matz added a commit that referenced this issue Feb 15, 2017

matz added a commit that referenced this issue Feb 15, 2017

Prohibit too deep `mrb_funcall()` recursion; ref #3421
`mrb_funcall()` recursion can cause stack overflow easily,
so recursion depth is now limited to MRB_FUNCALL_DEPTH_MAX,
which default value is 512.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment