From ead5f8b1f29bb298072e82c38b873d6bc8ccfef8 Mon Sep 17 00:00:00 2001 From: nick evans Date: Wed, 15 Apr 2026 18:31:51 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=A5=85=20Validate=20server's=20literal=20?= =?UTF-8?q?byte=20size=20format?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This guards against numbers like `99999999999999999999`. This isn't a security issue unless `max_response_size` is `nil`. But it's reasonable to block too large numbers in both the response reader and the response parser, regardless of that config setting. Note also that this still _allows_ strings like `000000000000000001`. This is goofy, but it's how the RFCs are written! --- lib/net/imap/response_parser.rb | 24 +++++++++---------- lib/net/imap/response_reader.rb | 7 +++++- .../response_parser/quirky_behaviors.yml | 16 +++++++++++++ test/net/imap/test_response_reader.rb | 21 ++++++++++++++++ 4 files changed, 55 insertions(+), 13 deletions(-) diff --git a/lib/net/imap/response_parser.rb b/lib/net/imap/response_parser.rb index 5fd6b40d..91f8d14c 100644 --- a/lib/net/imap/response_parser.rb +++ b/lib/net/imap/response_parser.rb @@ -2212,10 +2212,7 @@ def next_token if $1 return Token.new(T_SPACE, $+) elsif $2 - len = $+.to_i - val = @str[@pos, len] - @pos += len - return Token.new(T_LITERAL8, val) + literal_token($+, T_LITERAL8) elsif $3 && $7 # greedily match ATOM, prefixed with NUMBER, NIL, or PLUS. return Token.new(T_ATOM, $3) @@ -2243,10 +2240,7 @@ def next_token elsif $15 return Token.new(T_RBRA, $+) elsif $16 - len = $+.to_i - val = @str[@pos, len] - @pos += len - return Token.new(T_LITERAL, val) + literal_token($+) elsif $17 return Token.new(T_PERCENT, $+) elsif $18 @@ -2272,10 +2266,7 @@ def next_token elsif $4 return Token.new(T_QUOTED, Patterns.unescape_quoted($+)) elsif $5 - len = $+.to_i - val = @str[@pos, len] - @pos += len - return Token.new(T_LITERAL, val) + literal_token($+) elsif $6 return Token.new(T_LPAR, $+) elsif $7 @@ -2290,6 +2281,15 @@ def next_token else parse_error("invalid @lex_state - %s", @lex_state.inspect) end + rescue DataFormatError => error + parse_error error.message + end + + def literal_token(len, type = T_LITERAL) + len = NumValidator.coerce_number64 len + val = @str[@pos, len] + @pos += len + Token.new(type, val) end end diff --git a/lib/net/imap/response_reader.rb b/lib/net/imap/response_reader.rb index 139739d6..c689ff9e 100644 --- a/lib/net/imap/response_reader.rb +++ b/lib/net/imap/response_reader.rb @@ -4,6 +4,8 @@ module Net class IMAP # See https://www.rfc-editor.org/rfc/rfc9051#section-2.2.2 class ResponseReader # :nodoc: + include NumValidator + attr_reader :client def initialize(client, sock) @@ -46,7 +48,10 @@ def done? = line_done? && !literal_size def line_done? = buff.end_with?(CRLF) def get_literal_size(buff) - buff.end_with?("}\r\n") && buff.rindex(/\{(\d+)\}\r\n\z/n) && $1.to_i + buff.end_with?("}\r\n") && buff.rindex(/\{(\d+)\}\r\n\z/n) && + coerce_number64($1) + rescue DataFormatError + raise DataFormatError, format("invalid response literal size (%s)", $1) end def read_line diff --git a/test/net/imap/fixtures/response_parser/quirky_behaviors.yml b/test/net/imap/fixtures/response_parser/quirky_behaviors.yml index 9d626fb5..2b0dfef7 100644 --- a/test/net/imap/fixtures/response_parser/quirky_behaviors.yml +++ b/test/net/imap/fixtures/response_parser/quirky_behaviors.yml @@ -7,6 +7,22 @@ data: raw_data: "* NOOP\r\n" + "literal numeric formatted with zero-prefix": + :response: "* 20367 FETCH (BODY[HEADER.FIELDS (Foo)] {012}\r\nFoo: bar\r\n\r\n)\r\n" + :expected: !ruby/struct:Net::IMAP::UntaggedResponse + name: FETCH + data: !ruby/struct:Net::IMAP::FetchData + seqno: 20367 + attr: + BODY[HEADER.FIELDS (Foo)]: "Foo: bar\r\n\r\n" + raw_data: "* 20367 FETCH (BODY[HEADER.FIELDS (Foo)] {012}\r\nFoo: bar\r\n\r\n)\r\n" + + "invalid literal numeric format (too large)": + :test_type: :assert_parse_failure + :message: "number64 must be unsigned 63-bit integer: 99999999999999999999" + :response: + "* 20367 FETCH (BODY[] {99999999999999999999}\r\nwon't parse this)\r\n" + test_invalid_noop_response_with_unparseable_data: :response: "* NOOP froopy snood\r\n" :expected: !ruby/struct:Net::IMAP::IgnoredResponse diff --git a/test/net/imap/test_response_reader.rb b/test/net/imap/test_response_reader.rb index 3cb75763..49e0b8b3 100644 --- a/test/net/imap/test_response_reader.rb +++ b/test/net/imap/test_response_reader.rb @@ -25,6 +25,8 @@ def literal(str) = "{#{str.bytesize}}\r\n#{str}" zero_literal = "tag ok #{literal ""} #{literal ""}\r\n" illegal_crs = "tag ok #{many_crs} #{many_crs}\r\n" illegal_lfs = "tag ok #{literal "\r"}\n#{literal "\r"}\n\r\n" + zero_padded = "+ {010}\r\n1234567890\r\n" # NOTE: it's decimal, not octal! + goofy_zero = "+ {000}\r\n\r\n" io = StringIO.new([ simple, long_line, @@ -33,6 +35,8 @@ def literal(str) = "{#{str.bytesize}}\r\n#{str}" zero_literal, illegal_crs, illegal_lfs, + zero_padded, + goofy_zero, simple, ].join) rcvr = Net::IMAP::ResponseReader.new(client, io) @@ -43,6 +47,8 @@ def literal(str) = "{#{str.bytesize}}\r\n#{str}" assert_equal zero_literal, rcvr.read_response_buffer.to_str assert_equal illegal_crs, rcvr.read_response_buffer.to_str assert_equal illegal_lfs, rcvr.read_response_buffer.to_str + assert_equal zero_padded, rcvr.read_response_buffer.to_str + assert_equal goofy_zero, rcvr.read_response_buffer.to_str assert_equal simple, rcvr.read_response_buffer.to_str assert_equal "", rcvr.read_response_buffer.to_str end @@ -82,6 +88,21 @@ def literal(str) = "{#{str.bytesize}}\r\n#{str}" end end + data( + bad_int64: "+ {99999999999999999999}\r\ndon't even try to read this...", + ) + test "#read_response_buffer with invalid literal size" do |invalid| + client = FakeClient.new + client.config.max_response_size = nil # any size is allowed! + io = StringIO.new(invalid, "rb") + rcvr = Net::IMAP::ResponseReader.new(client, io) + assert_raise Net::IMAP::DataFormatError do + result = rcvr.read_response_buffer + flunk "Got result: %p" % [result] + end + # assert io.closed? + end + test "linear performance detecting literal continuation" do omit_unless_cruby "flaky on different platforms" omit_if(ENV["CI"], "slow and flaky, skipping in CI")