HexBrain

shopify-scripts · Report #200387

Incorrect code generation with redo inside NODE_RESCUE.

Validstatus
lowseverity
$100bounty
3upvotes
28 Feb 2017disclosed

How it was found

The following code causes mruby to use up all available memory:

class A redo rescue c end

Following the execution, we see the code in codegen.c jumping between CASE(OP_ONERR) and CASE(OP_JMP). CASE(OP_ONERR) uses realloc to double the size of mrb->c->rescue, and since it is stuck in an infinite loop between the two instructions, it runs out of memory.

The problematic jump is visible in the bytcode (emphasis added).

00001 NODE_SCOPE: 00001 NODE_BEGIN: 00001 NODE_CLASS: 00003 :A 00003 body: 00001 NODE_RESCUE: 00003 body: 00001 NODE_BEGIN: 00001 NODE_REDO 00003 rescue: 00003 handle classes: 00002 NODE_FCALL: 00002 NODE_SELF 00002 method='c' (670) 00003 rescue body: 00003 NODE_BEGIN:

irep 0x715200 nregs=3 nlocals=1 pools=0 syms=1 reps=1 1 000 OP_LOADNIL R1 1 001 OP_LOADNIL R2 1 002 OP_CLASS R1 :A 1 003 OP_EXEC R1 I(+1) 1 004 OP_STOP

irep 0x71b400 nregs=4 nlocals=1 pools=0 syms=2 reps=0 1 000 OP_ONERR 003 <------- Infinite loop 1 001 OP_JMP 000 <------- created here. 1 002 OP_JMP 013 1 003 OP_RESCUE R1 2 004 OP_LOADSELF R2 2 005 OP_SEND R2 :c 0 2 006 OP_MOVE R3 R1 2 007 OP_SEND R2 :=== 1 2 008 OP_JMPIF R2 010 2 009 OP_JMP 012 3 010 OP_LOADNIL R1 3 011 OP_JMP 014 3 012 OP_RAISE R1 3 013 OP_POPERR 1 3 014 OP_RETURN R1 return

Testing the same code with MRI Ruby shows that MRI Ruby rejects it as a syntax error. It would seem MRI Ruby forbids the use of redo in the context of rescue so the patch below disallows the related LOOP_* types. The test suite runs successfully with the patch. The bug is mitigated inside the mruby-engine sandbox because it triggers the instruction quota.

--- a/mrbgems/mruby-compiler/core/codegen.c
+++ b/mrbgems/mruby-compiler/core/codegen.c
@@ -2031,7 +2031,7 @@ codegen(codegen_scope *s, node *tree, int val)
     break;
 
   case NODE_REDO:
-    if (!s->loop) {
+    if (!s->loop || s->loop->type == LOOP_BEGIN || s->loop->type == LOOP_RESCUE) {
       raise_error(s, "unexpected redo");
     }
     else {

This report was disclosed publicly on HackerOne and belongs to its author and the program. See the original