Repository navigation
fix: raise RangeError on strings longer than INT_MAX - #762
Merged
Merged
Conversation
flavorjones
force-pushed
the
card-816-sql-query-truncation
branch
2 times, most recently
from
October 26, 2026 16:59
58ede54 to
72da0ea
Compare
When a custom aggregate's `finalize` returned a value that could not be converted to a SQLite result, such as a `Hash`, the exception escaped SQLite's xFinal callback unprotected, and a later GC could crash the process. Set the result inside the same `rb_protect` as the call to `finalize`.
SQL strings, bound String and Blob values, and String and Blob results of custom functions and aggregates were passed to SQLite with their length cast to `int`. A string of 4 GiB or more could have its length wrap to a smaller value, so SQLite read only a prefix and could drop trailing clauses from a query. Use `RSTRING_LENINT`, which raises `RangeError` instead. ref: GHSA-4fqr-96q8-xrfx
flavorjones
force-pushed
the
card-816-sql-query-truncation
branch
from
October 26, 2026 17:29
72da0ea to
f241dde
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
The C extension passes string lengths to SQLite as
(int)RSTRING_LEN(...)when preparing SQL (statement.c#L78), binding String and Blob values (statement.c#L281, #L292, #L303), and returning String and Blob results from custom functions and aggregates (database.c#L532, #L539). On 64-bit platforms a string of 4 GiB or more can have its length wrap to a smaller value, so SQLite reads only a prefix of the string.Details
Use
RSTRING_LENINT, which raisesRangeErrorwhen the length does not fit in anint.Converting an aggregate's result had a separate bug:
rb_sqlite3_aggregator_finalset the result outsiderb_protect(aggregator.c#L160-L166), so an exception there escaped SQLite's callback and a later GC crashed the process. Returning aHashfromfinalizetriggers it today. The first commit sets the result inside the samerb_protectas the call tofinalize.The tests use a string of 2^31 NUL bytes, which CRuby allocates with
calloc, so they run fast and use little memory. They skip wherelongis 32 bits, on TruffleRuby, under valgrind, and on FreeBSD, where I suspect they crashed a CI job.