Copy from the relocated string

When ungetting the string same as the same buffer string, extending the buffer can move the pointer in the argument. Reported by manun Manu (manun) at https://hackerone.com/reports/2805165.
ruby · Oct 29, 2024 · 95c1194 · 95c1194
1 parent ef03f93
commit 95c1194
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 9 deletions.
diff --git a/ext/stringio/stringio.c b/ext/stringio/stringio.c
@@ -930,6 +930,18 @@ strio_extend(struct StringIO *ptr, long pos, long len)
     }
 }
 
+static void
+strio_unget_string(struct StringIO *ptr, VALUE c)
+{
+    const char *cp = NULL;
+    long cl = RSTRING_LEN(c);
+    if (cl > 0) {
+	if (c != ptr->string) cp = RSTRING_PTR(c);
+	strio_unget_bytes(ptr, cp, cl);
+	RB_GC_GUARD(c);
+    }
+}
+
 /*
  * call-seq:
  *   ungetc(character) -> nil
@@ -967,8 +979,7 @@ strio_ungetc(VALUE self, VALUE c)
 	if (enc != enc2 && enc != rb_ascii8bit_encoding()) {
 	    c = rb_str_conv_enc(c, enc2, enc);
 	}
-	strio_unget_bytes(ptr, RSTRING_PTR(c), RSTRING_LEN(c));
-	RB_GC_GUARD(c);
+	strio_unget_string(ptr, c);
 	return Qnil;
     }
 }
@@ -995,13 +1006,8 @@ strio_ungetbyte(VALUE self, VALUE c)
 	strio_unget_bytes(ptr, &cc, 1);
     }
     else {
-	long cl;
 	StringValue(c);
-	cl = RSTRING_LEN(c);
-	if (cl > 0) {
-	    strio_unget_bytes(ptr, RSTRING_PTR(c), cl);
-	    RB_GC_GUARD(c);
-	}
+	strio_unget_string(ptr, c);
     }
     return Qnil;
 }
@@ -1032,7 +1038,7 @@ strio_unget_bytes(struct StringIO *ptr, const char *cp, long cl)
 	if (rest > cl) memset(s + len, 0, rest - cl);
 	pos -= cl;
     }
-    memcpy(s + pos, cp, cl);
+    memcpy(s + pos, (cp ? cp : s), cl);
     ptr->pos = pos;
     return Qnil;
 }

diff --git a/test/stringio/test_stringio.rb b/test/stringio/test_stringio.rb
@@ -842,6 +842,17 @@ def test_ungetc_fill
     assert_match(/\Ab+\z/, s.string)
   end
 
+  def test_ungetc_same_string
+    s = StringIO.new("abc" * 30)
+    s.ungetc(s.string)
+    assert_match(/\A(?:abc){60}\z/, s.string)
+
+    s = StringIO.new("abc" * 30)
+    s.pos = 70 # ("abc".size * 30 - 70).divmod(3) == [6, 2]
+    s.ungetc(s.string)
+    assert_match(/\A(?:abc){30}bc(?:abc){6}\z/, s.string)
+  end
+
   def test_ungetbyte_pos
     b = '\\b00010001 \\B00010001 \\b1 \\B1 \\b000100011'
     s = StringIO.new( b )
@@ -876,6 +887,17 @@ def test_ungetbyte_fill
     assert_match(/\Ab+\z/, s.string)
   end
 
+  def test_ungetbyte_same_string
+    s = StringIO.new("abc" * 30)
+    s.ungetc(s.string)
+    assert_match(/\A(?:abc){60}\z/, s.string)
+
+    s = StringIO.new("abc" * 30)
+    s.pos = 70 # ("abc".size * 30 - 70).divmod(3) == [6, 2]
+    s.ungetbyte(s.string)
+    assert_match(/\A(?:abc){30}bc(?:abc){6}\z/, s.string)
+  end
+
   def test_frozen
     s = StringIO.new
     s.freeze