From 641e3843419b7a6587c0d5a0562c022c97d31af1 Mon Sep 17 00:00:00 2001 From: nagachika 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 --- string.c | 20 ++++++++++++++++---- test/-ext-/string/test_fstring.rb | 9 +++++++++ version.h | 4 ++-- 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/string.c b/string.c index 07268f0bb5..6651dce2d0 100644 --- a/string.c +++ b/string.c @@ -74,6 +74,8 @@ VALUE rb_cSymbol; * 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; */ #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; 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) 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) 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 1b3b15c922..8b9eca891d 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 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 4136d36fac..76309cb794 100644 --- a/version.h +++ b/version.h @@ -1,10 +1,10 @@ #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 v1.2.3