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

ruby-changes:57984

From: nagachika <ko1@a...>
Date: Fri, 27 Sep 2019 20:23:38 +0900 (JST)
Subject: [ruby-changes:57984] 641e384341 (ruby_2_6): merge revision(s) 93faa011d393bb4b5cf31a0cbb46922f0a5e7cdc: [Backport #16151]

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

From 641e3843419b7a6587c0d5a0562c022c97d31af1 Mon Sep 17 00:00:00 2001
From: nagachika <nagachika@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>
Date: Fri, 27 Sep 2019 11:23:18 +0000
Subject: merge revision(s) 93faa011d393bb4b5cf31a0cbb46922f0a5e7cdc: [Backport
 #16151]

	Tag string shared roots to fix use-after-free

	The buffer deduplication codepath in rb_fstring can be used to free the buffer
	of shared string roots, which leads to use-after-free.

	Introudce a new flag to tag strings that at one point have been a shared root.
	Check for it in rb_fstring to avoid freeing buffers that are shared by
	multiple strings. This change is based on nobu's idea in [ruby-core:94838].

	The included test case test for the sequence of calls to internal functions
	that lead to this bug. See attached ticket for Ruby level repros.

	[Bug #16151]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_6@67804 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

diff --git a/string.c b/string.c
index 07268f0..6651dce 100644
--- a/string.c
+++ b/string.c
@@ -74,6 +74,8 @@ VALUE rb_cSymbol; https://github.com/ruby/ruby/blob/trunk/string.c#L74
  * 1:     RSTRING_NOEMBED
  * 2:     STR_SHARED (== ELTS_SHARED)
  * 2-6:   RSTRING_EMBED_LEN (5 bits == 32)
+ * 5:     STR_SHARED_ROOT (RSTRING_NOEMBED==1 && STR_SHARED == 0, there may be
+ *                         other strings that rely on this string's buffer)
  * 6:     STR_IS_SHARED_M (shared, when RSTRING_NOEMBED==1 && klass==0)
  * 7:     STR_TMPLOCK
  * 8-9:   ENC_CODERANGE (2 bits)
@@ -84,6 +86,7 @@ VALUE rb_cSymbol; https://github.com/ruby/ruby/blob/trunk/string.c#L86
  */
 
 #define RUBY_MAX_CHAR_LEN 16
+#define STR_SHARED_ROOT FL_USER5
 #define STR_IS_SHARED_M FL_USER6
 #define STR_TMPLOCK FL_USER7
 #define STR_NOFREE FL_USER18
@@ -157,6 +160,7 @@ VALUE rb_cSymbol; https://github.com/ruby/ruby/blob/trunk/string.c#L160
     if (!FL_TEST(str, STR_FAKESTR)) { \
 	RB_OBJ_WRITE((str), &RSTRING(str)->as.heap.aux.shared, (shared_str)); \
 	FL_SET((str), STR_SHARED); \
+        FL_SET((shared_str), STR_SHARED_ROOT); \
 	if (RBASIC_CLASS((shared_str)) == 0) /* for CoW-friendliness */ \
 	    FL_SET_RAW((shared_str), STR_IS_SHARED_M); \
     } \
@@ -318,9 +322,15 @@ rb_fstring(VALUE str) https://github.com/ruby/ruby/blob/trunk/string.c#L322
 	return str;
 
     bare = BARE_STRING_P(str);
-    if (STR_EMBED_P(str) && !bare) {
-	OBJ_FREEZE_RAW(str);
-	return str;
+    if (!bare) {
+        if (STR_EMBED_P(str)) {
+            OBJ_FREEZE_RAW(str);
+            return str;
+        }
+        if (FL_TEST_RAW(str, STR_NOEMBED|STR_SHARED_ROOT|STR_SHARED) == (STR_NOEMBED|STR_SHARED_ROOT)) {
+            assert(OBJ_FROZEN(str));
+            return str;
+        }
     }
 
     fstr = register_fstring(str);
@@ -1173,7 +1183,9 @@ str_replace_shared_without_enc(VALUE str2, VALUE str) https://github.com/ruby/ruby/blob/trunk/string.c#L1183
             RSTRING_GETMEM(root, ptr, len);
         }
         if (!STR_EMBED_P(str2) && !FL_TEST_RAW(str2, STR_SHARED|STR_NOFREE)) {
-            /* TODO: check if str2 is a shared root */
+            if (FL_TEST_RAW(str2, STR_SHARED_ROOT)) {
+                rb_fatal("about to free a possible shared root");
+            }
             char *ptr2 = STR_HEAP_PTR(str2);
             if (ptr2 != ptr) {
                 ruby_sized_xfree(ptr2, STR_HEAP_SIZE(str2));
diff --git a/test/-ext-/string/test_fstring.rb b/test/-ext-/string/test_fstring.rb
index 1b3b15c..8b9eca8 100644
--- a/test/-ext-/string/test_fstring.rb
+++ b/test/-ext-/string/test_fstring.rb
@@ -71,4 +71,13 @@ class Test_String_Fstring < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/-ext-/string/test_fstring.rb#L71
     str.freeze
     assert_fstring(str) {|s| assert_instance_of(S, s)}
   end
+
+  def test_shared_string_safety
+    -('a' * 30).force_encoding(Encoding::ASCII)
+    str = ('a' * 30).force_encoding(Encoding::ASCII).taint
+    frozen_str = Bug::String.rb_str_new_frozen(str)
+    assert_fstring(frozen_str) {|s| assert_equal(str, s)}
+    GC.start
+    assert_equal('a' * 30, str, "[Bug #16151]")
+  end
 end
diff --git a/version.h b/version.h
index 4136d36..76309cb 100644
--- a/version.h
+++ b/version.h
@@ -1,10 +1,10 @@ https://github.com/ruby/ruby/blob/trunk/version.h#L1
 #define RUBY_VERSION "2.6.5"
 #define RUBY_RELEASE_DATE RUBY_RELEASE_YEAR_STR"-"RUBY_RELEASE_MONTH_STR"-"RUBY_RELEASE_DAY_STR
-#define RUBY_PATCHLEVEL 108
+#define RUBY_PATCHLEVEL 109
 
 #define RUBY_RELEASE_YEAR 2019
 #define RUBY_RELEASE_MONTH 9
-#define RUBY_RELEASE_DAY 14
+#define RUBY_RELEASE_DAY 27
 
 #include "ruby/version.h"
 
-- 
cgit v0.10.2


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

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