RUBY-3840 Keep MemoryPointer alive in Binary.wrap_string#3063
Draft
comandeo-mongo wants to merge 1 commit into
Draft
RUBY-3840 Keep MemoryPointer alive in Binary.wrap_string#3063comandeo-mongo wants to merge 1 commit into
comandeo-mongo wants to merge 1 commit into
Conversation
mongocrypt_binary_new_from_data does not copy the buffer, so the FFI::MemoryPointer backing the wrapped binary must outlive the mongocrypt_binary_t. The pointer was an anonymous temporary with no Ruby-side reference, so GC could free its buffer while libmongocrypt still pointed into it - a use-after-free that manifests as crashes or corrupted BSON under GC pressure. Hold the pointer in a local for the lifetime of the method frame. Add a GC-pressure regression spec that reads the wrapped buffer back through libmongocrypt.
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
1 participant
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.
Problem
Mongo::Crypt::Binary.wrap_stringpassedFFI::MemoryPointer.from_string(str)directly intomongocrypt_binary_new_from_dataas an anonymous temporary. That C API does not copy the buffer — the binding doc states "the data is not copied and must outlive the mongocrypt_binary_t object". With no Ruby-side reference, theMemoryPointerbecame GC-eligible the moment the call returned. If GC ran during theyield, its finalizer freed the buffer while libmongocrypt still pointed into it — a use-after-free.Impact is intermittent (GC-timing dependent under MRI): crashes under GC pressure, garbled BSON / spurious libmongocrypt errors, or silent local corruption of the BSON fed to libmongocrypt (KMS providers, schema maps, key IDs/material, KMS network bytes). Not attacker-controlled, so a reliability bug rather than a security vulnerability.
Fix
Hold the
MemoryPointerin a local for the lifetime of the method frame, mirroring howBinary#initializeretains its pointer in@data_p.Files changed
lib/mongo/crypt/binary.rb— retain the pointer in a local across the wrapped binary's lifetime.spec/mongo/crypt/binary_spec.rb— add a.wrap_stringgroup, including a GC-pressure regression test that reads the wrapped buffer back through libmongocrypt.Test plan
spec/mongo/crypt/binary_spec.rb— new GC-pressure test fails on the old code (libmongocrypt reads freed/reused bytes), passes with the fix. 11 examples, 0 failures.spec/mongo/crypt/handle_spec.rb+binary_spec.rb— 81 examples, 0 failures (cloud-KMS cases pending without credentials).data_key_spec.rb,explicit_encryption_spec.rb) — remaining failures are pre-existing and environment-only (AWS credentials / KMIP mock server); identical with and without this change.bundle exec rubocop— clean.RUBY-3840