fix: raise RangeError on strings longer than INT_MAX

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ## next / unreleased
 
+### Security / Stability
+
+- Raise `RangeError` when a SQL string, a bound String or Blob value, or a String or Blob returned from a custom function or aggregate is longer than `INT_MAX` bytes. Previously the length was truncated to 32 bits, so SQLite could read only part of the string and drop trailing clauses from a query. See [GHSA-4fqr-96q8-xrfx](https://github.com/sparklemotion/sqlite3-ruby/security/advisories/GHSA-4fqr-96q8-xrfx) for more information. #762 @flavorjones
+- Fix a crash when a custom aggregate's `finalize` returns a value that can't be converted to a SQLite result. The exception is now raised from the query and the connection remains usable. #762 @flavorjones
+
 ### Improved
 
 - `Statement#step` no longer allocates and discards an array when the statement has no more rows. This saves one object per statement execution. #758 @thomasklemm

ext/sqlite3/aggregator.c

@@ -25,6 +25,7 @@ typedef struct rb_sqlite3_protected_funcall_args {
     ID method;
     int argc;
     VALUE *params;
+    sqlite3_context *result_ctx;
 } protected_funcall_args_t;
 
 /* why isn't there something like this in the ruby API? */
@@ -34,15 +35,21 @@ rb_sqlite3_protected_funcall_body(VALUE protected_funcall_args_ptr)
     protected_funcall_args_t *args =
         (protected_funcall_args_t *)protected_funcall_args_ptr;
 
-    return rb_funcall2(args->self, args->method, args->argc, args->params);
+    VALUE result = rb_funcall2(args->self, args->method, args->argc, args->params);
+
+    if (args->result_ctx) {
+        set_sqlite3_func_result(args->result_ctx, result);
+    }
+
+    return result;
 }
 
 static VALUE
 rb_sqlite3_protected_funcall(VALUE self, ID method, int argc, VALUE *params,
-                             int *exc_status)
+                             sqlite3_context *result_ctx, int *exc_status)
 {
     protected_funcall_args_t args = {
-        .self = self, .method = method, .argc = argc, .params = params
+        .self = self, .method = method, .argc = argc, .params = params, .result_ctx = result_ctx
     };
     return rb_protect(rb_sqlite3_protected_funcall_body, (VALUE)(&args), exc_status);
 }
@@ -71,7 +78,7 @@ rb_sqlite3_aggregate_instance(sqlite3_context *ctx)
 
         inst = rb_class_new_instance(0, NULL, cAggregatorInstance);
         rb_iv_set(inst, "-handler_instance", rb_sqlite3_protected_funcall(
-                      handler_klass, rb_intern("new"), 0, NULL, &exc_status));
+                      handler_klass, rb_intern("new"), 0, NULL, NULL, &exc_status));
         rb_iv_set(inst, "-exc_status", INT2NUM(exc_status));
 
         rb_ary_push(instances, inst);
@@ -141,7 +148,7 @@ rb_sqlite3_aggregator_step(sqlite3_context *ctx, int argc, sqlite3_value **argv)
         }
     }
     rb_sqlite3_protected_funcall(
-        handler_instance, rb_intern("step"), argc, params, &exc_status);
+        handler_instance, rb_intern("step"), argc, params, NULL, &exc_status);
     if (argc > 1) {
         ALLOCV_END(params_handle);
     }
@@ -158,11 +165,8 @@ rb_sqlite3_aggregator_final(sqlite3_context *ctx)
     int exc_status = NUM2INT(rb_iv_get(inst, "-exc_status"));
 
     if (!exc_status) {
-        VALUE result = rb_sqlite3_protected_funcall(
-                           handler_instance, rb_intern("finalize"), 0, NULL, &exc_status);
-        if (!exc_status) {
-            set_sqlite3_func_result(ctx, result);
-        }
+        rb_sqlite3_protected_funcall(
+            handler_instance, rb_intern("finalize"), 0, NULL, ctx, &exc_status);
     }
 
     if (exc_status) {

ext/sqlite3/database.c

@@ -529,14 +529,14 @@ set_sqlite3_func_result(sqlite3_context *ctx, VALUE result)
                 sqlite3_result_blob(
                     ctx,
                     (const void *)StringValuePtr(result),
-                    (int)RSTRING_LEN(result),
+                    RSTRING_LENINT(result),
                     SQLITE_TRANSIENT
                 );
             } else {
                 sqlite3_result_text(
                     ctx,
                     (const char *)StringValuePtr(result),
-                    (int)RSTRING_LEN(result),
+                    RSTRING_LENINT(result),
                     SQLITE_TRANSIENT
                 );
             }

ext/sqlite3/statement.c

@@ -75,7 +75,7 @@ prepare(VALUE self, VALUE db, VALUE sql)
 #endif
                  db_ctx->db,
                  (const char *)StringValuePtr(sql),
-                 (int)RSTRING_LEN(sql),
+                 RSTRING_LENINT(sql),
                  &ctx->st,
                  &tail
              );
@@ -279,7 +279,7 @@ bind_param(VALUE self, VALUE key, VALUE value)
                              ctx->st,
                              index,
                              (const char *)StringValuePtr(value),
-                             (int)RSTRING_LEN(value),
+                             RSTRING_LENINT(value),
                              SQLITE_TRANSIENT
                          );
             } else {
@@ -290,7 +290,7 @@ bind_param(VALUE self, VALUE key, VALUE value)
                                  ctx->st,
                                  index,
                                  (const char *)StringValuePtr(value),
-                                 (int)RSTRING_LEN(value),
+                                 RSTRING_LENINT(value),
                                  SQLITE_TRANSIENT
                              );
                 } else {
@@ -301,7 +301,7 @@ bind_param(VALUE self, VALUE key, VALUE value)
                                  ctx->st,
                                  index,
                                  (const char *)StringValuePtr(value),
-                                 (int)RSTRING_LEN(value),
+                                 RSTRING_LENINT(value),
                                  SQLITE_TRANSIENT
                              );
                 }

test/helper.rb

@@ -127,6 +127,16 @@ def skip_if_timing_unreliable
     def windows?
       ::RUBY_PLATFORM =~ /mingw|mswin/
     end
+
+    # SQLite takes string lengths as int, so it can't be passed a string longer than INT_MAX bytes.
+    # CRuby allocates a string of NUL bytes with calloc, which stays cheap until it's read.
+    def string_longer_than_int_max(encoding = Encoding::UTF_8)
+      skip("a string can't be longer than INT_MAX bytes on this platform") if [0].pack("l!").bytesize < 8
+      skip("only CRuby allocates this string cheaply") if RUBY_ENGINE != "ruby"
+      skip("valgrind's calloc writes every byte") if i_am_running_in_valgrind
+      skip("suspected of crashing FreeBSD CI, see #762") if RUBY_PLATFORM.include?("freebsd")
+      ("\0" * (2**31)).force_encoding(encoding)
+    end
   end
 end
 

test/test_database.rb

@@ -585,6 +585,22 @@ def test_function_return_types
       end
     end
 
+    def test_function_raises_on_returned_text_longer_than_int_max
+      text = string_longer_than_int_max
+      @db.define_function("huge") { |a| text }
+
+      assert_raise(RangeError) { @db.execute("select huge(1)") }
+      assert_equal([[2]], @db.execute("select 1 + 1"))
+    end
+
+    def test_function_raises_on_returned_blob_longer_than_int_max
+      blob = string_longer_than_int_max(Encoding::BINARY)
+      @db.define_function("huge") { |a| blob }
+
+      assert_raise(RangeError) { @db.execute("select huge(1)") }
+      assert_equal([[2]], @db.execute("select 1 + 1"))
+    end
+
     def test_function_raise_propagates_and_connection_remains_usable
       @db.define_function("boom") { |a| raise "boom: #{a}" }
 

test/test_integration_aggregate.rb

@@ -207,6 +207,29 @@ def test_create_aggregate_with_exception_in_finalize
     end
   end
 
+  def test_create_aggregate_raises_on_returned_unsupported_type
+    @db.create_aggregate("unsupported", 1) do
+      step { |ctx, a| }
+      finalize { |ctx| ctx.result = {} }
+    end
+
+    assert_raise(RuntimeError) { @db.get_first_value("select unsupported(a) from foo") }
+    assert_equal(2, @db.get_first_value("select 1 + 1"))
+    GC.start
+  end
+
+  def test_create_aggregate_raises_on_returned_text_longer_than_int_max
+    text = string_longer_than_int_max
+    @db.create_aggregate("huge", 1) do
+      step { |ctx, a| }
+      finalize { |ctx| ctx.result = text }
+    end
+
+    assert_raise(RangeError) { @db.get_first_value("select huge(a) from foo") }
+    assert_equal(2, @db.get_first_value("select 1 + 1"))
+    GC.start
+  end
+
   def test_create_aggregate_with_no_data
     @db.create_aggregate("accumulate", 1) do
       step do |ctx, a|

test/test_statement.rb

@@ -223,6 +223,32 @@ def test_bind_blob
       end
     end
 
+    def test_prepare_raises_on_sql_longer_than_int_max
+      sql = string_longer_than_int_max
+      assert_raises(RangeError) { SQLite3::Statement.new(@db, sql) }
+    end
+
+    def test_bind_param_raises_on_text_longer_than_int_max
+      text = string_longer_than_int_max
+      stmt = SQLite3::Statement.new(@db, "select ?")
+      assert_raises(RangeError) { stmt.bind_param(1, text) }
+      stmt.close
+    end
+
+    def test_bind_param_raises_on_utf16_text_longer_than_int_max
+      text = string_longer_than_int_max(Encoding::UTF_16LE)
+      stmt = SQLite3::Statement.new(@db, "select ?")
+      assert_raises(RangeError) { stmt.bind_param(1, text) }
+      stmt.close
+    end
+
+    def test_bind_param_raises_on_blob_longer_than_int_max
+      blob = string_longer_than_int_max(Encoding::BINARY)
+      stmt = SQLite3::Statement.new(@db, "select ?")
+      assert_raises(RangeError) { stmt.bind_param(1, blob) }
+      stmt.close
+    end
+
     def test_decltype_is_utf8
       @db.execute %(create table foo(a integer, b "VARCHÄR(255)"))
       @db.prepare("select a, b from foo") do |stmt|