[Bug #21787] IO::Buffer: Check addition overflows

https://hackerone.com/reports/3437743

Author: nobu

io_buffer.c

@@ -1526,13 +1526,19 @@ VALUE rb_io_buffer_free_locked(VALUE self)
     return self;
 }
 
+static bool
+size_sum_is_bigger_than(size_t a, size_t b, size_t x)
+{
+    struct rbimpl_size_mul_overflow_tag size = rbimpl_size_add_overflow(a, b);
+    return size.left || size.right > x;
+}
+
 // Validate that access to the buffer is within bounds, assuming you want to
 // access length bytes from the specified offset.
 static inline void
 io_buffer_validate_range(struct rb_io_buffer *buffer, size_t offset, size_t length)
 {
-    // We assume here that offset + length won't overflow:
-    if (offset + length > buffer->size) {
+    if (size_sum_is_bigger_than(offset, length, buffer->size)) {
         rb_raise(rb_eArgError, "Specified offset+length is bigger than the buffer size!");
     }
 }
@@ -1842,9 +1848,9 @@ rb_io_buffer_compare(VALUE self, VALUE other)
 }
 
 static void
-io_buffer_validate_type(size_t size, size_t offset)
+io_buffer_validate_type(size_t size, size_t offset, size_t extend)
 {
-    if (offset > size) {
+    if (size_sum_is_bigger_than(offset, extend, size)) {
         rb_raise(rb_eArgError, "Type extends beyond end of buffer! (offset=%"PRIdSIZE" > size=%"PRIdSIZE")", offset, size);
     }
 }
@@ -1944,7 +1950,7 @@ static ID RB_IO_BUFFER_DATA_TYPE_##name; \
 static VALUE \
 io_buffer_read_##name(const void* base, size_t size, size_t *offset) \
 { \
-    io_buffer_validate_type(size, *offset + sizeof(type)); \
+    io_buffer_validate_type(size, *offset, sizeof(type)); \
     type value; \
     memcpy(&value, (char*)base + *offset, sizeof(type)); \
     if (endian != RB_IO_BUFFER_HOST_ENDIAN) value = swap(value); \
@@ -1955,7 +1961,7 @@ io_buffer_read_##name(const void* base, size_t size, size_t *offset) \
 static void \
 io_buffer_write_##name(const void* base, size_t size, size_t *offset, VALUE _value) \
 { \
-    io_buffer_validate_type(size, *offset + sizeof(type)); \
+    io_buffer_validate_type(size, *offset, sizeof(type)); \
     type value = unwrap(_value); \
     if (endian != RB_IO_BUFFER_HOST_ENDIAN) value = swap(value); \
     memcpy((char*)base + *offset, &value, sizeof(type)); \
@@ -2483,7 +2489,7 @@ io_buffer_memmove(struct rb_io_buffer *buffer, size_t offset, const void *source
 
     io_buffer_validate_range(buffer, offset, length);
 
-    if (source_offset + length > source_size) {
+    if (size_sum_is_bigger_than(source_offset, length, source_size)) {
         rb_raise(rb_eArgError, "The computed source range exceeds the size of the source buffer!");
     }
 

test/ruby/test_io_buffer.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: false
 
 require 'tempfile'
+require 'rbconfig/sizeof'
 
 class TestIOBuffer < Test::Unit::TestCase
   experimental = Warning[:experimental]
@@ -414,6 +415,8 @@ def test_zero_length_get_string
     :F64 => [-1.0, 0.0, 0.5, 1.0, 128.0],
   }
 
+  SIZE_MAX = RbConfig::LIMITS["SIZE_MAX"]
+
   def test_get_set_value
     buffer = IO::Buffer.new(128)
 
@@ -422,6 +425,16 @@ def test_get_set_value
         buffer.set_value(data_type, 0, value)
         assert_equal value, buffer.get_value(data_type, 0), "Converting #{value} as #{data_type}."
       end
+      assert_raise(ArgumentError) {buffer.get_value(data_type, 128)}
+      assert_raise(ArgumentError) {buffer.set_value(data_type, 128, 0)}
+      case data_type
+      when :U8, :S8
+      else
+        assert_raise(ArgumentError) {buffer.get_value(data_type, 127)}
+        assert_raise(ArgumentError) {buffer.set_value(data_type, 127, 0)}
+        assert_raise(ArgumentError) {buffer.get_value(data_type, SIZE_MAX)}
+        assert_raise(ArgumentError) {buffer.set_value(data_type, SIZE_MAX, 0)}
+      end
     end
   end
 
@@ -503,7 +516,7 @@ def test_clear
     assert_raise(ArgumentError) {buffer.clear(0, 20)}
     assert_raise(ArgumentError) {buffer.clear(0, 0, 20)}
     assert_raise(ArgumentError) {buffer.clear(0, 10, 10)}
-    assert_raise(ArgumentError) {buffer.clear(0, (1<<64)-8, 10)}
+    assert_raise(ArgumentError) {buffer.clear(0, SIZE_MAX-7, 10)}
   end
 
   def test_invalidation