fix(rube): reject negative counts and stop discarding name nodes

Codex, working the defensive half without having written the parser, probed M1
with adversarial input and found defects my own tests missed. Verified
independently before fixing, and two more were found while confirming:

  negative_array_count   ACCEPTED as an empty array
  negative_hash_count    ACCEPTED as an empty hash
  negative_bignum_words  NoMethodError leaked outside StreamError
  negative_ivar_count    ACCEPTED
  negative_string_len    cursor moved BACKWARD, wrong error raised

The last is the worst. take(-5) does not trip the count > remaining guard, so
byteslice returns nil and @position decreases. A parser whose cursor can rewind
on attacker input is a loop primitive, not merely a wrong error.

The M1 gate claimed bignum length confusion was covered. It was not. Oversized
widths were tested and negative counts never were, because the same author
chose both the implementation and the cases it would face. That is the
negative-control failure one level up, and it is exactly what an author cannot
catch alone.

Fixes: every count and length now flows through read_count with a role label
and a nonnegative check, take rejects negative byte counts outright, and
MalformedCountError joins the StreamError hierarchy so nothing leaks a raw
NoMethodError. Negative link indices were already guarded; regression tests now
pin that.

Also closes a detection blind spot Codex identified. read_ivar and read_object
discarded the parsed name nodes after taking their values, so a sink tag placed
in an instance-variable-name position vanished from Result#sinks. Node now
carries an auxiliary collection that Node#each traverses, and name and class
nodes are retained. Proven: a stream with a userdef tag in the name position
now reports Evil#_load where it previously reported nothing.

46 parser tests, 80 assertions. All controls pass, exploit gate still passes.

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 = {

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

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
 

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
 

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) }