diff --git a/lib/mongo/crypt/binary.rb b/lib/mongo/crypt/binary.rb index 2322b9a1b5..71407f80af 100644 --- a/lib/mongo/crypt/binary.rb +++ b/lib/mongo/crypt/binary.rb @@ -139,14 +139,17 @@ def ref # Wraps a String with a mongocrypt_binary_t, yielding an FFI::Pointer # to the wrapped struct. def self.wrap_string(str) - binary_p = Binding.mongocrypt_binary_new_from_data( - FFI::MemoryPointer.from_string(str), - str.bytesize - ) + # mongocrypt_binary_new_from_data does not copy the buffer, so the + # MemoryPointer must outlive the mongocrypt_binary_t. Hold it in a + # local so it stays referenced for the whole method frame; otherwise + # GC can free the buffer while libmongocrypt still points into it. + data_p = FFI::MemoryPointer.from_string(str) + binary_p = Binding.mongocrypt_binary_new_from_data(data_p, str.bytesize) begin yield binary_p ensure Binding.mongocrypt_binary_destroy(binary_p) + data_p # rubocop:disable Lint/Void -- keep the buffer alive past destroy end end end diff --git a/spec/mongo/crypt/binary_spec.rb b/spec/mongo/crypt/binary_spec.rb index 73f140bc65..255c6c4d1a 100644 --- a/spec/mongo/crypt/binary_spec.rb +++ b/spec/mongo/crypt/binary_spec.rb @@ -80,6 +80,35 @@ end end + describe '#self.wrap_string' do + it 'yields a binary that reads back the original data' do + described_class.wrap_string(data) do |binary_p| + str_p = Mongo::Crypt::Binding.get_binary_data_direct(binary_p) + len = Mongo::Crypt::Binding.get_binary_len_direct(binary_p) + expect(str_p.read_string(len)).to eq(data) + end + end + + it 'keeps the wrapped buffer valid under GC pressure' do + # mongocrypt_binary_new_from_data does not copy the buffer, so the + # MemoryPointer backing the wrapped binary must stay referenced for the + # whole block. If it is collected, GC frees the buffer and the bytes + # libmongocrypt sees get corrupted. Force GC and allocation churn inside + # the block to surface a use-after-free. + 100.times do |i| + str = "wrap-string-payload-#{i}-#{'x' * 64}" + 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 } + str_p = Mongo::Crypt::Binding.get_binary_data_direct(binary_p) + len = Mongo::Crypt::Binding.get_binary_len_direct(binary_p) + expect(str_p.read_string(len)).to eq(str) + end + end + end + end + describe '#write' do # Binary must have enough space pre-allocated let(:binary) { described_class.from_data("\00" * data.length) }