diff --git a/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/constants.rb b/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/constants.rb index 2d0f1ec9..8f0302ba 100644 --- a/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/constants.rb +++ b/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/constants.rb @@ -49,6 +49,13 @@ module Rube DEFAULT_MAX_DEPTH = 256 + ROLE_LENGTH = "byte length" + ROLE_ARRAY = "array element" + ROLE_HASH = "hash entry" + ROLE_IVAR = "instance variable" + ROLE_STRUCT = "struct member" + ROLE_BIGNUM = "bignum word" + SINK_TAGS = [TAG_USERDEF, TAG_USERMARSHAL, TAG_DATA].freeze SINK_METHODS = { diff --git a/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/errors.rb b/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/errors.rb index 5817ad95..4b3cd95b 100644 --- a/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/errors.rb +++ b/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/errors.rb @@ -16,5 +16,7 @@ module Rube class InvalidLinkError < StreamError; end class DepthLimitError < StreamError; end + + class MalformedCountError < StreamError; end end end diff --git a/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/node.rb b/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/node.rb index 34fa1f03..eb51757a 100644 --- a/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/node.rb +++ b/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/node.rb @@ -4,7 +4,7 @@ module Rube module Marshal class Node - attr_reader :type, :tag, :children, :instance_variables_map + attr_reader :type, :tag, :children, :instance_variables_map, :auxiliary attr_accessor :value, :class_name def initialize(type:, tag: nil, value: nil, class_name: nil) @@ -14,6 +14,7 @@ module Rube @class_name = class_name @children = [] @instance_variables_map = {} + @auxiliary = [] end def sink? @@ -34,6 +35,7 @@ module Rube yield self children.each { |child| child.each(&block) } instance_variables_map.each_value { |child| child.each(&block) } + auxiliary.each { |child| child.each(&block) } end end diff --git a/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/parser.rb b/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/parser.rb index 1eafff04..c7a7afe1 100644 --- a/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/parser.rb +++ b/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/parser.rb @@ -31,6 +31,7 @@ module Rube end def take(count) + raise MalformedCountError, "negative byte count #{count}" if count.negative? raise TruncatedStreamError, "wanted #{count} bytes, had #{remaining}" if count > remaining slice = source.byteslice(@position, count) @@ -38,6 +39,13 @@ module Rube slice end + def read_count(role) + value = read_fixnum + raise MalformedCountError, "negative #{role} count #{value}" if value.negative? + + value + end + def take_byte take(1).unpack1("C") end @@ -73,7 +81,7 @@ module Rube end def read_counted_bytes - take(read_fixnum) + take(read_count(ROLE_LENGTH)) end def register(node) @@ -136,7 +144,7 @@ module Rube def read_bignum(tag) negative = take(1) == BIGNUM_SIGN_NEGATIVE - magnitude = little_endian(take(read_fixnum * BIGNUM_WORD_BYTES)) + magnitude = little_endian(take(read_count(ROLE_BIGNUM) * BIGNUM_WORD_BYTES)) Node.new(type: :bignum, tag: tag, value: negative ? -magnitude : magnitude) end @@ -158,13 +166,13 @@ module Rube def read_array(tag, depth) node = register(Node.new(type: :array, tag: tag)) - read_fixnum.times { node.children << read_value(depth + 1) } + read_count(ROLE_ARRAY).times { node.children << read_value(depth + 1) } node end def read_hash(tag, depth) node = register(Node.new(type: :hash, tag: tag)) - read_fixnum.times { node.children << read_pair(depth) } + read_count(ROLE_HASH).times { node.children << read_pair(depth) } node.children << read_value(depth + 1) if tag == TAG_HASH_DEFAULT node end @@ -178,8 +186,9 @@ module Rube def read_ivar(tag, depth) inner = read_value(depth) - read_fixnum.times do + read_count(ROLE_IVAR).times do name = read_value(depth + 1) + inner.auxiliary << name inner.instance_variables_map[name.value] = read_value(depth + 1) end inner @@ -187,9 +196,12 @@ module Rube def read_object(tag, depth) node = register(Node.new(type: :object, tag: tag)) - node.class_name = read_value(depth + 1).value.to_s - read_fixnum.times do + class_node = read_value(depth + 1) + node.class_name = class_node.value.to_s + node.auxiliary << class_node + read_count(ROLE_IVAR).times do name = read_value(depth + 1) + node.auxiliary << name node.instance_variables_map[name.value] = read_value(depth + 1) end node @@ -198,7 +210,7 @@ module Rube def read_struct(tag, depth) node = register(Node.new(type: :struct, tag: tag)) node.class_name = read_value(depth + 1).value.to_s - read_fixnum.times { node.children << read_pair(depth) } + read_count(ROLE_STRUCT).times { node.children << read_pair(depth) } node end diff --git a/PROJECTS/beginner/deserialization-gadget-lab/test/marshal/parser_test.rb b/PROJECTS/beginner/deserialization-gadget-lab/test/marshal/parser_test.rb index 4ce6c90f..8d2d1463 100644 --- a/PROJECTS/beginner/deserialization-gadget-lab/test/marshal/parser_test.rb +++ b/PROJECTS/beginner/deserialization-gadget-lab/test/marshal/parser_test.rb @@ -159,6 +159,66 @@ module Rube assert_raises(TruncatedStreamError) { parse("\x04\x08i\x03\x01") } end + def test_rejects_negative_array_count + assert_raises(MalformedCountError) { parse("\x04\x08[\xFA") } + end + + def test_rejects_negative_hash_count + assert_raises(MalformedCountError) { parse("\x04\x08{\xFA") } + end + + def test_rejects_negative_bignum_word_count + assert_raises(MalformedCountError) { parse("\x04\x08l+\xFA") } + end + + def test_rejects_negative_instance_variable_count + assert_raises(MalformedCountError) { parse("\x04\x08I\"\x06a\xFA") } + end + + def test_rejects_negative_string_length + assert_raises(MalformedCountError) { parse("\x04\x08\"\xFA") } + end + + def test_rejects_negative_struct_member_count + assert_raises(MalformedCountError) { parse("\x04\x08S:\x06A\xFA") } + end + + def test_negative_counts_never_rewind_the_cursor + parser = Parser.new("\x04\x08\"\xFA") + assert_raises(MalformedCountError) { parser.parse } + end + + def test_every_malformed_count_stays_inside_the_stream_error_hierarchy + ["\x04\x08[\xFA", "\x04\x08{\xFA", "\x04\x08l+\xFA", "\x04\x08\"\xFA"].each do |blob| + parse(blob) + flunk "expected #{blob.inspect} to be rejected" + rescue StreamError + pass + rescue StandardError => e + flunk "#{blob.inspect} leaked #{e.class} outside StreamError" + end + end + + def test_rejects_negative_object_link_index + assert_raises(InvalidLinkError) { parse("\x04\x08[\x06@\xFA") } + end + + def test_rejects_negative_symlink_index + assert_raises(InvalidLinkError) { parse("\x04\x08[\x07:\x06a;\xFA") } + end + + def test_sink_in_an_instance_variable_name_position_is_still_reported + result = parse("\x04\x08I\"\x06a\x06u:\x09Evil\x06x0") + assert_includes result.class_names, "Evil" + assert_equal ["Evil#_load"], result.sinks.map { |s| "#{s.class_name}##{s.sink_method}" } + end + + def test_class_name_node_is_traversable + result = parse(::Marshal.dump(Fixture.new)) + symbols = result.nodes.select { |n| n.type == :symbol } + refute_empty symbols + end + def test_rejects_depth_beyond_limit deep = "\x04\x08" + ("[\x06" * (Constants::DEFAULT_MAX_DEPTH + 5)) + "0" assert_raises(DepthLimitError) { parse(deep) }