[GR-18163] Fix Marshal.load when dump is "too short"

PullRequest: truffleruby/3872

Author: andrykonchin

CHANGELOG.md

@@ -7,6 +7,7 @@ Bug fixes:
 
 * Fix `Dir.glob` returning blank string entry with leading `**/` in glob and `base:` argument (@rwstauner).
 * Fix class lookup after an object's class has been replaced by `IO#reopen` (@itarato, @eregon).
+* Fix `Marshal.load` and raise `ArgumentError` when dump is broken and is too short (#3108, @andrykonchin).
 
 Compatibility:
 

spec/ruby/core/marshal/shared/load.rb

@@ -623,6 +623,14 @@
       value.map(&:encoding).should == [Encoding::UTF_8, Encoding::UTF_8, Encoding::UTF_8]
       value.should == [*expected, expected[0]]
     end
+
+    it "raises ArgumentError when end of byte sequence reached before symbol characters end" do
+      Marshal.dump(:hello).should == "\x04\b:\nhello"
+
+      -> {
+        Marshal.send(@method, "\x04\b:\nhel")
+      }.should raise_error(ArgumentError, "marshal data too short")
+    end
   end
 
   describe "for a String" do
@@ -685,6 +693,14 @@ def io.binmode; raise "binmode"; end
       result.encoding.should == Encoding::BINARY
       result.should == str
     end
+
+    it "raises ArgumentError when end of byte sequence reached before string characters end" do
+      Marshal.dump("hello").should == "\x04\b\"\nhello"
+
+      -> {
+        Marshal.send(@method, "\x04\b\"\nhel")
+      }.should raise_error(ArgumentError, "marshal data too short")
+    end
   end
 
   describe "for a Struct" do
@@ -819,6 +835,14 @@ def io.binmode; raise "binmode"; end
         Marshal.send(@method, "\x04\bo:\tFile\001\001:\001\005@path\"\x10/etc/passwd")
       end.should raise_error(ArgumentError)
     end
+
+    it "raises ArgumentError when end of byte sequence reached before class name end" do
+      Marshal.dump(Object.new).should == "\x04\bo:\vObject\x00"
+
+      -> {
+        Marshal.send(@method, "\x04\bo:\vObj")
+      }.should raise_error(ArgumentError, "marshal data too short")
+    end
   end
 
   describe "for an object responding to #marshal_dump and #marshal_load" do
@@ -908,6 +932,14 @@ def io.binmode; raise "binmode"; end
       regexp.encoding.should == Encoding::UTF_32LE
       regexp.source.should == "a".encode("utf-32le")
     end
+
+    it "raises ArgumentError when end of byte sequence reached before source string end" do
+      Marshal.dump(/hello world/).should == "\x04\bI/\x10hello world\x00\x06:\x06EF"
+
+      -> {
+        Marshal.send(@method, "\x04\bI/\x10hel")
+      }.should raise_error(ArgumentError, "marshal data too short")
+    end
   end
 
   describe "for a Float" do
@@ -929,6 +961,14 @@ def io.binmode; raise "binmode"; end
       obj = 1.1867345e+22
       Marshal.send(@method, "\004\bf\0361.1867344999999999e+22\000\344@").should == obj
     end
+
+    it "raises ArgumentError when end of byte sequence reached before float string representation end" do
+      Marshal.dump(1.3).should == "\x04\bf\b1.3"
+
+      -> {
+        Marshal.send(@method, "\004\bf\v1")
+      }.should raise_error(ArgumentError, "marshal data too short")
+    end
   end
 
   describe "for an Integer" do
@@ -1114,6 +1154,14 @@ def io.binmode; raise "binmode"; end
     it "raises ArgumentError if given a nonexistent class" do
       -> { Marshal.send(@method, "\x04\bc\vStrung") }.should raise_error(ArgumentError)
     end
+
+    it "raises ArgumentError when end of byte sequence reached before class name end" do
+      Marshal.dump(String).should == "\x04\bc\vString"
+
+      -> {
+        Marshal.send(@method, "\x04\bc\vStr")
+      }.should raise_error(ArgumentError, "marshal data too short")
+    end
   end
 
   describe "for a Module" do
@@ -1128,6 +1176,14 @@ def io.binmode; raise "binmode"; end
     it "loads an old module" do
       Marshal.send(@method, "\x04\bM\vKernel").should == Kernel
     end
+
+    it "raises ArgumentError when end of byte sequence reached before module name end" do
+      Marshal.dump(Kernel).should == "\x04\bm\vKernel"
+
+      -> {
+        Marshal.send(@method, "\x04\bm\vKer")
+      }.should raise_error(ArgumentError, "marshal data too short")
+    end
   end
 
   describe "for a wrapped C pointer" do

src/main/ruby/truffleruby/core/marshal.rb

@@ -1387,8 +1387,10 @@ def construct_range(klass)
   end
 
   class IOState < State
-    def consume(bytes)
-      @stream.read(bytes)
+    def consume(bytes_count)
+      string = @stream.read(bytes_count)
+      raise ArgumentError, 'marshal data too short' if string.bytesize < bytes_count
+      string
     end
 
     def consume_byte
@@ -1411,11 +1413,11 @@ def inspect
       "#<Marshal::StringState #{@stream[@consumed..-1].inspect}>"
     end
 
-    def consume(bytes)
-      raise ArgumentError, 'marshal data too short' if @consumed > @stream.bytesize
-      data = @stream.byteslice @consumed, bytes
-      @consumed += bytes
-      data
+    def consume(bytes_count)
+      raise ArgumentError, 'marshal data too short' if @consumed + bytes_count > @stream.bytesize
+      string = @stream.byteslice @consumed, bytes_count
+      @consumed += bytes_count
+      string
     end
 
     def consume_byte