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

Use after free during range creation #3387

Closed
clayton-shopify opened this Issue Jan 10, 2017 · 0 comments

Comments

Projects
None yet
1 participant
@clayton-shopify
Contributor

clayton-shopify commented Jan 10, 2017

The following input demonstrates a crash (at least when MRuby is built on Ubuntu 14.04):

a=[]..[]
b=[]
c=[]
d=[]
e %w(0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0)

This outputs the following:

-:5: undefined method 'e' for main (NoMethodError)
mruby: malloc.c:3987: _int_free: Assertion `p->fd_nextsize->bk_nextsize == p' failed.
Aborted (core dumped)

This issue was original reported by https://hackerone.com/icanthack

Later https://hackerone.com/titanous reported what appears to be the same issue, and provided this crashing input (again, confirmed to crash MRuby when built on Ubuntu 14.04):

[][0,0]..[1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]

He provided the following diagnosis:

range_check can trigger a stack extension via mrb_funcall if there are many arguments passed to the equivalence test. The stack extension changes the address of the stack, though the old stack address is used when assigning the value of the range. This causes a write into the old heap allocation, which has already been freed.

and suggested the following patch:

From ec7d6b4078f25bcc7c25b210e2d69c910ea9b923 Mon Sep 17 00:00:00 2001
From: Jonathan Rudenberg <jonathan@titanous.com>
Date: Fri, 30 Dec 2016 17:44:25 -0500
Subject: [PATCH] Fix heap use-after-free during range creation

---
 src/vm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/vm.c b/src/vm.c
index cca0fd03..368b75a9 100644
--- a/src/vm.c
+++ b/src/vm.c
@@ -2411,7 +2411,8 @@ RETRY_TRY_BLOCK:
     CASE(OP_RANGE) {
       /* A B C  R(A) := range_new(R(B),R(B+1),C) */
       int b = GETARG_B(i);
-      regs[GETARG_A(i)] = mrb_range_new(mrb, regs[b], regs[b+1], GETARG_C(i));
+      mrb_value res = mrb_range_new(mrb, regs[b], regs[b+1], GETARG_C(i));
+      regs[GETARG_A(i)] = res;
       ARENA_RESTORE(mrb, ai);
       NEXT;
     }
-- 
2.11.0

@matz matz closed this in db1bd07 Jan 11, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment