[前][次][番号順一覧][スレッド一覧]

ruby-changes:58169

From: =E5=8D=9C=E9=83=A8=E6=98=8C=E5=B9=B3 <ko1@a...>
Date: Wed, 9 Oct 2019 12:12:47 +0900 (JST)
Subject: [ruby-changes:58169] a14cc07f2f (master): avoid returning NULL from xrealloc

https://git.ruby-lang.org/ruby.git/commit/?id=a14cc07f2f

From a14cc07f2ffc704b73ba4b96543e2f85c3ed1921 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E5=8D=9C=E9=83=A8=E6=98=8C=E5=B9=B3?=
 <shyouhei@r...>
Date: Tue, 8 Oct 2019 16:07:31 +0900
Subject: avoid returning NULL from xrealloc

This changeset is to kill future possibility of bugs similar to
CVE-2019-11932.   The vulnerability occurs when reallocarray(3)
(which is a variant of realloc(3) and roughly resembles our
ruby_xmalloc2()) returns NULL.  In our C API, ruby_xmalloc()
never returns NULL to raise NoMemoryError instead.  ruby_xfree()
does not return NULL by definition.  ruby_xrealloc() on the other
hand, _did_ return NULL, _and_ also raised sometimes.  It is very
confusing.  Let's not do that.  x-series APIs shall raise on
error and shall not return NULL.

diff --git a/gc.c b/gc.c
index 2ff9094..9a33da0 100644
--- a/gc.c
+++ b/gc.c
@@ -9927,8 +9927,41 @@ objspace_xrealloc(rb_objspace_t *objspace, void *ptr, size_t new_size, size_t ol https://github.com/ruby/ruby/blob/trunk/gc.c#L9927
      * see http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_400.htm
      */
     if (new_size == 0) {
-	objspace_xfree(objspace, ptr, old_size);
-	return 0;
+        if ((mem = objspace_xmalloc0(objspace, 0)) != NULL) {
+            /*
+             * - OpenBSD's malloc(3) man page says that when 0 is passed, it
+             *   returns a non-NULL pointer to an access-protected memory page.
+             *   The returned pointer cannot be read / written at all, but
+             *   still be a valid argument of free().
+             *
+             *   https://man.openbsd.org/malloc.3
+             *
+             * - Linux's malloc(3) man page says that it _might_ perhaps return
+             *   a non-NULL pointer when its argument is 0.  That return value
+             *   is safe (and is expected) to be passed to free().
+             *
+             *   http://man7.org/linux/man-pages/man3/malloc.3.html
+             *
+             * - As I read the implementation jemalloc's malloc() returns fully
+             *   normal 16 bytes memory region when its argument is 0.
+             *
+             * - As I read the implementation musl libc's malloc() returns
+             *   fully normal 32 bytes memory region when its argument is 0.
+             *
+             * - Other malloc implementations can also return non-NULL.
+             */
+            objspace_xfree(objspace, ptr, old_size);
+            return mem;
+        }
+        else {
+            /*
+             * It is dangerous to return NULL here, because that could lead to
+             * RCE.  Fallback to 1 byte instead of zero.
+             *
+             * https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-11932
+             */
+            new_size = 1;
+        }
     }
 
 #if CALC_EXACT_MALLOC_SIZE
@@ -10016,6 +10049,13 @@ rb_malloc_info_show_results(void) https://github.com/ruby/ruby/blob/trunk/gc.c#L10049
 static void
 objspace_xfree(rb_objspace_t *objspace, void *ptr, size_t old_size)
 {
+    if (!ptr) {
+        /*
+         * ISO/IEC 9899 says "If ptr is a null pointer, no action occurs" since
+         * its first version.  We would better follow.
+         */
+        return;
+    }
 #if CALC_EXACT_MALLOC_SIZE
     struct malloc_obj_info *info = (struct malloc_obj_info *)ptr - 1;
     ptr = info;
-- 
cgit v0.10.2


--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/

[前][次][番号順一覧][スレッド一覧]