From: XrXr@... Date: 2019-09-23T00:25:27+00:00 Subject: [ruby-core:95040] [Ruby master Bug#16151] [PATCH] Fix a class of fstring related use-after-free Issue #16151 has been updated by alanwu (Alan Wu). I ran some quick tests on rejecting frozen non-bare strings from the buffer deduplication code path. On Discourse, about 18% of calls to rb_fstring are refused with the change in https://bugs.ruby-lang.org/issues/16151#note-5. So I don't think that's a viable approach. I've put together a PR with nobu's idea, using a new flag to make sure to not defeat the optimization in #13085. I also took the liberty to rename STR_IS_SHARED_M since it's a little confusing to have this many things talk about sharing. https://github.com/ruby/ruby/pull/2480 ---------------------------------------- Bug #16151: [PATCH] Fix a class of fstring related use-after-free https://bugs.ruby-lang.org/issues/16151#change-81671 * Author: alanwu (Alan Wu) * Status: Open * Priority: Normal * Assignee: * Target version: * ruby -v: ruby 2.7.0dev (2019-09-07T18:26:35Z master e9bc8b35c6) [x86_64-linux] * Backport: 2.5: UNKNOWN, 2.6: UNKNOWN ---------------------------------------- Pull request: https://github.com/ruby/ruby/pull/2435 ## The bug Run the following against master(e9bc8b3) to observe use-after-free: ```ruby -('a' * 30).force_encoding(Encoding::ASCII) a = ('a' * 30).force_encoding(Encoding::ASCII).taint t = Thread.new{} t.name = a eval('', binding, t.name) p a ``` ```ruby -('a' * 30).force_encoding(Encoding::ASCII) a = ('a' * 30).force_encoding(Encoding::ASCII).taint require 'ripper' ripper = Ripper.new("", a) eval('', binding, ripper.filename) p a ``` There may be other cases in the standard library or in the wild. ## Background When a string has both `STR_NOEMBED` and `STR_SHARED` set, it relies on a different string for its buffer. I will refer to strings that are depended upon as "shared roots". Shared roots are frozen and have the `STR_SHARED` unset. This is a bit unintuitive to me. A name for `STR_SHARED` that makes more sense to me would be `STR_BUFFER_ELSEWHERE`. ## What went wrong It is not safe to free the buffer of a shared root while it has dependants. The root and its dependants use the same buffer. As such, it is only safe to free the shared buffer when all users are unreachable on the heap. ## The Fix `rb_fstring` has a code path that frees and replaces the buffer of its input. Using this code path on the shared root of dependant strings sets up use-after-free. This patch removes the problematic code path as no tests require said buffer replacement functionality. Additionally, there has been three other issues that steam from this particular code path. See #15926, #15916 and #16136 --- I used @mame's commit in #16136 as the starting point for this investigation. Thank you! -- https://bugs.ruby-lang.org/ Unsubscribe: