THRIFT-6131: Bound Ruby HeaderTransport varint32 parsing

lib/rb/lib/thrift/transport/header_transport.rb

@@ -89,6 +89,9 @@ class HeaderTransport < BaseTransport
     COMPACT_VERSION_MASK = 0x1f
     COMPACT_VERSION = 0x01
 
+    MAX_VARINT32_BYTES = 5
+    MAX_VARINT32_LAST_BYTE = 0x0f
+
     attr_reader :protocol_id, :sequence_id, :flags
 
     # Creates a new HeaderTransport wrapping the given transport.
@@ -500,11 +503,20 @@ def read_varint32(io, boundary_pos = nil)
         end
         byte = io.getbyte
         raise TransportException.new(TransportException::END_OF_FILE, "Unexpected EOF reading varint") if byte.nil?
+
+        if shift == (MAX_VARINT32_BYTES - 1) * 7
+          if (byte & 0x80) != 0
+            raise TransportException.new(TransportException::UNKNOWN, "Variable-length int over 5 bytes.")
+          end
+          if (byte & ~MAX_VARINT32_LAST_BYTE) != 0
+            raise TransportException.new(TransportException::UNKNOWN, "Variable-length int overflows uint32.")
+          end
+        end
+
         result |= (byte & 0x7f) << shift
-        break if (byte & 0x80) == 0
+        return result if (byte & 0x80) == 0
         shift += 7
       end
-      result
     end
 
     # Writes a varint32 to the given IO

lib/rb/spec/header_transport_spec.rb

@@ -320,6 +320,34 @@
         expect { read_trans.read(1) }.to raise_error(Thrift::TransportException, /header boundary/)
       end
 
+      it "should reject varints longer than uint32" do
+        header_data = "\x80".b * 65_532
+        frame = build_header_frame(header_data)
+        read_transport = Thrift::MemoryBufferTransport.new(frame)
+        read_trans = Thrift::HeaderTransport.new(read_transport)
+
+        expect { read_trans.read(1) }.to raise_error(Thrift::TransportException, /over 5 bytes/)
+      end
+
+      it "should reject uint32 varints with an overflowing fifth byte" do
+        header_data = ([0x80] * 4 + [0x10]).pack('C*')
+        frame = build_header_frame(header_data)
+        read_transport = Thrift::MemoryBufferTransport.new(frame)
+        read_trans = Thrift::HeaderTransport.new(read_transport)
+
+        expect { read_trans.read(1) }.to raise_error(Thrift::TransportException, /overflows uint32/)
+      end
+
+      it "should accept uint32 varints with a valid fifth byte" do
+        header_data = ([0x80] * 4 + [0x0f, 0]).pack('C*')
+        frame = build_header_frame(header_data)
+        read_transport = Thrift::MemoryBufferTransport.new(frame)
+        read_trans = Thrift::HeaderTransport.new(read_transport)
+
+        expect(read_trans.read(1)).to eq(Thrift::Bytes.empty_byte_buffer)
+        expect(read_trans.protocol_id).to eq(0xf0000000)
+      end
+
       it "should reject strings that exceed header boundary" do
         header_data = +""
         header_data << varint32(Thrift::HeaderSubprotocolID::BINARY)