diff --git a/CHANGELOG.md b/CHANGELOG.md index a3ffa10..c7ded10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,20 @@ ## 1.5.0 +* Fixed a denial-of-service issue in the decoder. A crafted database could nest + data-section pointers to shared targets so that decoding one record cost + exponential time and memory from a small file. The decoder now limits the + number of values it decodes for a single record and rejects a database that + exceeds it, along with pointer cycles and over-deep data, with an + `InvalidDatabaseError`. See GHSA-hj94-g986-h9r7. +* Fixed a related payload-amplification denial of service. A crafted database + could point many times at one large string or bytes value, so a record with + few values still materialized gigabytes. The decoder now also limits the + total string and bytes payload it decodes for a single record to 2 MiB, + charged as each value is decoded so a re-decoded target recharges, and + rejects an excess with the same `InvalidDatabaseError`. Both limits also + guard the metadata decoded when a database is opened. See + GHSA-hj94-g986-h9r7. * Unnecessary files were removed from the published .gem. ## 1.4.0 (2025-11-20) diff --git a/lib/maxmind/db/decoder.rb b/lib/maxmind/db/decoder.rb index 179f665..c0ea150 100644 --- a/lib/maxmind/db/decoder.rb +++ b/lib/maxmind/db/decoder.rb @@ -32,32 +32,101 @@ def initialize(io, pointer_base = 0, pointer_test = false) end # rubocop:enable Style/OptionalBooleanParameter + # Per-lookup limits recommended by the MaxMind DB specification. +budget+ + # is a three-element array, [values_remaining, depth, bytes_remaining], + # shared across the recursion so every count survives it. It is call-local, + # which keeps the decoder safe for concurrent reads. + # + # The value limit stops a pointer fan-out: each array and map subtracts its + # declared value count before iterating, so a re-decoded node drains the + # budget, and an oversized declared size is rejected before the loop reads + # anything. The largest real records decode a few hundred values. + # + # The byte limit stops payload amplification: a crafted database can point + # many times at one large string or bytes value, so a bounded value count + # still materializes gigabytes. Each string and bytes value, and each + # variable-length integer, subtracts its own length before it is read, so a + # re-decoded (fanned-out) target recharges its payload and an oversized + # declared length is rejected before any bytes are copied. Fixed-width + # scalars are not charged. The limit matches libmaxminddb and the Go reader. + # + # The depth limit stops a pointer cycle or over-deep data before the stack + # overflows. + MAX_VALUES = 1 << 16 + private_constant :MAX_VALUES + + MAX_BYTES = 1 << 21 + private_constant :MAX_BYTES + + MAX_DEPTH = 512 + private_constant :MAX_DEPTH + + # JRuby can exhaust the stack before the depth limit is reached and raises + # a Java StackOverflowError, which is not a SystemStackError. Catch both so + # a pointer cycle always becomes an InvalidDatabaseError. + STACK_ERRORS = if defined?(JRUBY_VERSION) + [SystemStackError, Java::JavaLang::StackOverflowError].freeze + else + [SystemStackError].freeze + end + private_constant :STACK_ERRORS + private - def decode_array(size, offset) + def descend(budget) + budget[1] += 1 + return unless budget[1] > MAX_DEPTH + + raise InvalidDatabaseError, + 'The MaxMind DB file\'s data section exceeds the maximum depth' + end + + def enter_container(values, budget) + if (budget[0] -= values).negative? + raise InvalidDatabaseError, + 'The MaxMind DB file\'s data section exceeds the maximum number of values' + end + descend(budget) + end + + # Charge +size+ bytes against the payload budget before the bytes are read. + # Charging first means an oversized declared length is rejected before it + # is copied. Ruby integers are arbitrary precision, so the subtraction + # cannot overflow. + def charge_bytes(size, budget) + return unless (budget[2] -= size).negative? + + raise InvalidDatabaseError, + 'The MaxMind DB file\'s data section exceeds the maximum number of bytes' + end + + def decode_array(size, offset, budget) + enter_container(size, budget) array = [] size.times do - value, offset = decode(offset) + value, offset = decode_with_budget(offset, budget) array << value end + budget[1] -= 1 [array, offset] end - def decode_boolean(size, offset) + def decode_boolean(size, offset, _budget) [size != 0, offset] end - def decode_bytes(size, offset) + def decode_bytes(size, offset, budget) + charge_bytes(size, budget) [@io.read(offset, size), offset + size] end - def decode_double(size, offset) + def decode_double(size, offset, _budget) verify_size(8, size) buf = @io.read(offset, 8) [buf.unpack1('G'), offset + 8] end - def decode_float(size, offset) + def decode_float(size, offset, _budget) verify_size(4, size) buf = @io.read(offset, 4) [buf.unpack1('g'), offset + 4] @@ -70,33 +139,35 @@ def verify_size(expected, actual) 'The MaxMind DB file\'s data section contains bad data (unknown data type or corrupt data)' end - def decode_int32(size, offset) - decode_int('l>', 4, size, offset) + def decode_int32(size, offset, budget) + decode_int('l>', 4, size, offset, budget) end - def decode_uint16(size, offset) - decode_int('n', 2, size, offset) + def decode_uint16(size, offset, budget) + decode_int('n', 2, size, offset, budget) end - def decode_uint32(size, offset) - decode_int('N', 4, size, offset) + def decode_uint32(size, offset, budget) + decode_int('N', 4, size, offset, budget) end - def decode_uint64(size, offset) - decode_int('Q>', 8, size, offset) + def decode_uint64(size, offset, budget) + decode_int('Q>', 8, size, offset, budget) end - def decode_int(type_code, type_size, size, offset) + def decode_int(type_code, type_size, size, offset, budget) return 0, offset if size == 0 + charge_bytes(size, budget) buf = @io.read(offset, size) buf = buf.rjust(type_size, "\x00") if size != type_size [buf.unpack1(type_code), offset + size] end - def decode_uint128(size, offset) + def decode_uint128(size, offset, budget) return 0, offset if size == 0 + charge_bytes(size, budget) buf = @io.read(offset, size) if size <= 8 @@ -112,17 +183,20 @@ def decode_uint128(size, offset) [a | b, offset + size] end - def decode_map(size, offset) + def decode_map(size, offset, budget) + # A map entry decodes a key and a value, so it costs two values. + enter_container(size * 2, budget) container = {} size.times do - key, offset = decode(offset) - value, offset = decode(offset) + key, offset = decode_with_budget(offset, budget) + value, offset = decode_with_budget(offset, budget) container[key] = value end + budget[1] -= 1 [container, offset] end - def decode_pointer(size, offset) + def decode_pointer(size, offset, budget) pointer_size = size >> 3 case pointer_size @@ -146,11 +220,18 @@ def decode_pointer(size, offset) return pointer, new_offset if @pointer_test - value, = decode(pointer) + if (budget[0] -= 1).negative? + raise InvalidDatabaseError, + 'The MaxMind DB file\'s data section exceeds the maximum number of values' + end + descend(budget) + value, = decode_with_budget(pointer, budget) + budget[1] -= 1 [value, new_offset] end - def decode_utf8_string(size, offset) + def decode_utf8_string(size, offset, budget) + charge_bytes(size, budget) new_offset = offset + size buf = @io.read(offset, size) buf.force_encoding(Encoding::UTF_8) @@ -187,6 +268,22 @@ def decode_utf8_string(size, offset) # # Throws an exception if there is an error. def decode(offset) + # Bound the work per lookup so a crafted database cannot exhaust CPU or + # memory. +budget+ carries the remaining value count, the current depth, + # and the remaining string and bytes payload, and is call-local, which + # keeps the decoder safe for concurrent reads. The depth limit catches a + # pointer cycle on MRI. JRuby can exhaust the stack before the limit is + # reached and raises a Java StackOverflowError, so catch that too and + # report the same error. + decode_with_budget(offset, [MAX_VALUES, 0, MAX_BYTES]) + rescue *STACK_ERRORS + raise InvalidDatabaseError, + 'The MaxMind DB file\'s data section exceeds the maximum depth' + end + + private + + def decode_with_budget(offset, budget) new_offset = offset + 1 buf = @io.read(offset, 1) ctrl_byte = buf.ord @@ -196,11 +293,9 @@ def decode(offset) size, new_offset = size_from_ctrl_byte(ctrl_byte, new_offset, type_num) # We could check an element exists at `type_num', but for performance I # don't. - send(TYPE_DECODER[type_num], size, new_offset) + send(TYPE_DECODER[type_num], size, new_offset, budget) end - private - def read_extended(offset) buf = @io.read(offset, 1) next_byte = buf.ord diff --git a/test/data b/test/data index b019327..d692a4b 160000 --- a/test/data +++ b/test/data @@ -1 +1 @@ -Subproject commit b019327b2c96a4efe08a9aa20c9e73150d104147 +Subproject commit d692a4b74c68c6e856d0bd85a38ee405b65c816f diff --git a/test/test_decoder.rb b/test/test_decoder.rb index 72de891..c0878a6 100644 --- a/test/test_decoder.rb +++ b/test/test_decoder.rb @@ -129,6 +129,68 @@ def test_pointer validate_type_decoding('pointers', pointers) end + def encode_pointer1(target) + # One-byte-payload pointer (type 1, pointer_size 0) with base 0. + [(1 << 5) | ((target >> 8) & 0x7), target & 0xFF].pack('C*').b + end + + def test_pointer_fan_out_is_bounded + # A data section of nested arrays, each holding two pointers to the node + # below, would cost 2**depth decode operations. The decoder bounds the + # number of values it decodes per lookup and rejects the database. + depth = 100 + buf = "\xa0".b # leaf: uint16 with value 0 + prev = 0 + depth.times do + offset = buf.bytesize + buf += "\x02\x04".b + encode_pointer1(prev) + encode_pointer1(prev) + prev = offset + end + + io = MaxMind::DB::MemoryReader.new(buf, is_buffer: true) + assert_raises(MaxMind::DB::InvalidDatabaseError) do + MaxMind::DB::Decoder.new(io, 0).decode(prev) + end + end + + def test_flat_scalar_pointer_fan_out_counts_pointer_targets + # The array declaration accounts for each pointer field. Following each + # pointer must also account for the scalar target that it decodes. + pointer_count = 32_769 + array_header = [0x1e, 4, pointer_count - 285].pack('CCn') + array = array_header + (encode_pointer1(0) * pointer_count) + io = MaxMind::DB::MemoryReader.new("\xa0".b + array, is_buffer: true) + + error = assert_raises(MaxMind::DB::InvalidDatabaseError) do + MaxMind::DB::Decoder.new(io, 0).decode(1) + end + assert_equal( + 'The MaxMind DB file\'s data section exceeds the maximum number of values', + error.message + ) + end + + def test_cyclic_pointer_raises + # A pointer to itself must raise a catchable InvalidDatabaseError rather + # than recursing until the interpreter's stack overflows. + io = MaxMind::DB::MemoryReader.new("\x20\x00".b, is_buffer: true) + assert_raises(MaxMind::DB::InvalidDatabaseError) do + MaxMind::DB::Decoder.new(io, 0).decode(0) + end + end + + def test_oversized_map_is_bounded + # A map entry decodes a key and a value, so a map of N entries costs 2N + # values. A map that declares 32,769 entries reaches 65,538 values, just + # past the 65,536 limit, and is rejected before any entry is read. 0xfe is + # a map with size code 30, then the two size bytes for 32,769 - 285 = 32,484 + # (0x7ee4). + io = MaxMind::DB::MemoryReader.new("\xfe\x7e\xe4".b, is_buffer: true) + assert_raises(MaxMind::DB::InvalidDatabaseError) do + MaxMind::DB::Decoder.new(io, 0).decode(0) + end + end + # rubocop:disable Style/ClassVars @@strings = { "\x40".b => '', diff --git a/test/test_reader.rb b/test/test_reader.rb index aafa9a4..779b7dc 100644 --- a/test/test_reader.rb +++ b/test/test_reader.rb @@ -241,6 +241,67 @@ def test_broken_database reader.close end + def test_payload_amplification_is_bounded + # Each database resolves every lookup to a record that points many times at + # one large string or bytes value. Following each pointer would copy the + # target again, so a reader that materializes every occurrence produces far + # more data than the file holds. The decoder rejects each one instead. The + # -dos and -string fixtures stay under the value count and are stopped by + # the payload byte budget; the -worst-case fixture holds enough pointers + # that this reader's value accounting stops it first. Both outcomes reject + # the record rather than expand it. + names = %w[ + MaxMind-DB-test-payload-amplification-dos + MaxMind-DB-test-payload-amplification-dos-string + MaxMind-DB-test-payload-amplification-dos-worst-case + ] + modes = [MaxMind::DB::MODE_FILE, MaxMind::DB::MODE_MEMORY] + names.each do |name| + modes.each do |mode| + reader = MaxMind::DB.new("test/data/test-data/#{name}.mmdb", mode: mode) + assert_raises(MaxMind::DB::InvalidDatabaseError, "#{name} (#{mode})") do + reader.get('1.1.1.1') + end + reader.close + end + end + end + + def test_payload_byte_budget_boundary + # The at-limit fixture materializes exactly 2 MiB of payload and must + # decode. The over-limit fixture holds one byte more and must be rejected, + # so an off-by-one in the byte budget is caught. + reader = MaxMind::DB.new( + 'test/data/test-data/MaxMind-DB-test-decoder-payload-limit.mmdb' + ) + + refute_nil(reader.get('1.1.1.1')) + reader.close + + reader = MaxMind::DB.new( + 'test/data/test-data/MaxMind-DB-test-decoder-payload-limit-over.mmdb' + ) + e = assert_raises MaxMind::DB::InvalidDatabaseError do + reader.get('1.1.1.1') + end + assert_equal( + 'The MaxMind DB file\'s data section exceeds the maximum number of bytes', + e.message, + ) + reader.close + end + + def test_metadata_payload_amplification_is_bounded + # The languages metadata array points many times at one large string. + # Opening the database decodes the metadata, so the same budget must reject + # it there rather than materialize the amplified payload. + assert_raises MaxMind::DB::InvalidDatabaseError do + MaxMind::DB.new( + 'test/data/test-data/MaxMind-DB-test-metadata-payload-limit.mmdb' + ) + end + end + def test_ip_validation reader = MaxMind::DB.new( 'test/data/test-data/MaxMind-DB-test-decoder.mmdb'