RUBY-3840 Keep MemoryPointer alive in Binary.wrap_string#3063
Merged
comandeo-mongo merged 1 commit intoJun 17, 2026
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.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a GC-related use-after-free in Mongo::Crypt::Binary.wrap_string by ensuring the FFI::MemoryPointer backing the wrapped mongocrypt_binary_t remains strongly referenced for the duration of the yielded block.
Changes:
- Retain the
FFI::MemoryPointerin a local inBinary.wrap_stringto keep the underlying buffer alive while libmongocrypt references it. - Add specs for
.wrap_string, including a regression test that applies GC/allocation pressure and validates the wrapped bytes can be read back via libmongocrypt.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/mongo/crypt/binary.rb | Keeps the wrapped string buffer alive across the wrap_string yield to avoid GC-triggered UAF. |
| spec/mongo/crypt/binary_spec.rb | Adds .wrap_string coverage and a GC-pressure regression test validating buffer stability. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+100
to
+103
| described_class.wrap_string(str) do |binary_p| | ||
| GC.start(full_mark: true, immediate_sweep: true) | ||
| # Allocate garbage to reuse any freed buffer in this tick. | ||
| Array.new(1000) { 'y' * 64 } |
jamis
approved these changes
Jun 17, 2026
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.
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