From 1a70932b92b4d4d1bdaed3e46e3de082ee87b692 Mon Sep 17 00:00:00 2001 From: nick evans Date: Fri, 22 May 2026 15:27:20 -0400 Subject: [PATCH 01/20] =?UTF-8?q?=F0=9F=8D=92=20pick=20e224fd67:=20?= =?UTF-8?q?=F0=9F=A5=85=20Validate=20that=20Atom=20and=20Flag=20are=20not?= =?UTF-8?q?=20empty?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Atom` and `Flag` have only been used for argument validation since v0.6.4 (as well as v0.5.14 and v0.4.24), and they validated for absense of `atom-specials`. But they failed to check that the strings are not empty. While this could be used to create syntax errors, I don't believe it amounts a security vulnerability. The result would be no different from any other `BAD` server response, which an application must be prepared to handle. --- lib/net/imap/command_data.rb | 2 ++ test/net/imap/test_command_data.rb | 2 ++ 2 files changed, 4 insertions(+) diff --git a/lib/net/imap/command_data.rb b/lib/net/imap/command_data.rb index 0ace387b..e8f31297 100644 --- a/lib/net/imap/command_data.rb +++ b/lib/net/imap/command_data.rb @@ -262,6 +262,8 @@ def validate or raise DataFormatError, "#{self.class} must be ASCII only" data.match?(ResponseParser::Patterns::ATOM_SPECIALS) \ and raise DataFormatError, "#{self.class} must not contain atom-specials" + data.empty? \ + and raise DataFormatError, "#{self.class} must not be empty" end def send_data(imap, tag) diff --git a/test/net/imap/test_command_data.rb b/test/net/imap/test_command_data.rb index f476afdc..d6c193fe 100644 --- a/test/net/imap/test_command_data.rb +++ b/test/net/imap/test_command_data.rb @@ -109,6 +109,7 @@ def send_data(*data, tag: TAG) "with_quoted_specials\\", "with\rCR", "with\nLF", + "", # empty ].each do |symbol| assert_raise_with_message(Net::IMAP::DataFormatError, /\batom\b/i) do imap.send_data Atom[symbol] @@ -140,6 +141,7 @@ def send_data(*data, tag: TAG) :"with_quoted_specials\\", :"with\rCR", :"with\nLF", + :"", # empty ].each do |symbol| assert_raise_with_message(Net::IMAP::DataFormatError, /\bflag\b/i) do imap.send_data Flag[symbol] From 9e69af53b27c0b3ed3064fe647db5850c1778bfa Mon Sep 17 00:00:00 2001 From: nick evans Date: Fri, 8 May 2026 17:26:47 -0400 Subject: [PATCH 02/20] =?UTF-8?q?=F0=9F=8D=92=20pick=205c213ab7=20(#675):?= =?UTF-8?q?=20=F0=9F=8F=B7=EF=B8=8F=20Allow=2064-bit=20Integer=20arguments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, integer arguments had to be 32-bit unsigned, and a special case exception was made for the number in a `["CHANGEDSINCE", number]` array. But several other command arguments can be larger: `UNCHANGEDSINCE`, quota `resource-limit`, and `tagged-ext-simple`. So, generic Integer arguments should allow 64 bit numbers. We can add specific argument validation where it makes sense to do so. --- lib/net/imap/command_data.rb | 15 ++++++++------- test/net/imap/test_imap.rb | 20 +++++++++++++------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/lib/net/imap/command_data.rb b/lib/net/imap/command_data.rb index e8f31297..c674f168 100644 --- a/lib/net/imap/command_data.rb +++ b/lib/net/imap/command_data.rb @@ -14,14 +14,15 @@ def validate_data(data) when nil when String when Integer - NumValidator.ensure_number(data) + # Covers modseq-valzer, which is the largest valid IMAP integer + if data.negative? + raise DataFormatError, "Integer argument must be unsigned: #{data}" + elsif 0xffff_ffff_ffff_ffff < data + raise DataFormatError, "Integer argument must fit in 64 bits: #{data}" + end when Array - if data[0] == 'CHANGEDSINCE' - NumValidator.ensure_mod_sequence_value(data[1]) - else - data.each do |i| - validate_data(i) - end + data.each do |i| + validate_data(i) end when Time, Date, DateTime when Symbol diff --git a/test/net/imap/test_imap.rb b/test/net/imap/test_imap.rb index 79e6deaf..678c46b5 100644 --- a/test/net/imap/test_imap.rb +++ b/test/net/imap/test_imap.rb @@ -632,19 +632,23 @@ def test_send_invalid_number sock.print("RUBY0001 OK TEST completed\r\n") sock.gets # Integer: 2**32 - 1 sock.print("RUBY0002 OK TEST completed\r\n") - sock.gets # MessageSet: 1 + sock.gets # Integer: 2**32 sock.print("RUBY0003 OK TEST completed\r\n") - sock.gets # MessageSet: 2**32 - 1 + sock.gets # Integer: 2**64 - 1 sock.print("RUBY0004 OK TEST completed\r\n") - sock.gets # SequenceSet: -1 => "*" + sock.gets # MessageSet: 1 sock.print("RUBY0005 OK TEST completed\r\n") - sock.gets # SequenceSet: 1 + sock.gets # MessageSet: 2**32 - 1 sock.print("RUBY0006 OK TEST completed\r\n") - sock.gets # SequenceSet: 2**32 - 1 + sock.gets # SequenceSet: -1 => "*" sock.print("RUBY0007 OK TEST completed\r\n") + sock.gets # SequenceSet: 1 + sock.print("RUBY0008 OK TEST completed\r\n") + sock.gets # SequenceSet: 2**32 - 1 + sock.print("RUBY0009 OK TEST completed\r\n") sock.gets # LOGOUT sock.print("* BYE terminating connection\r\n") - sock.print("RUBY0008 OK LOGOUT completed\r\n") + sock.print("RUBY0010 OK LOGOUT completed\r\n") ensure sock.close server.close @@ -658,8 +662,10 @@ def test_send_invalid_number end imap.__send__(:send_command, "TEST", 0) imap.__send__(:send_command, "TEST", 2**32 - 1) + imap.__send__(:send_command, "TEST", 2**32) + imap.__send__(:send_command, "TEST", 2**64 - 1) assert_raise(Net::IMAP::DataFormatError) do - imap.__send__(:send_command, "TEST", 2**32) + imap.__send__(:send_command, "TEST", 2**64) end # MessageSet numbers may be non-zero uint32 assert_raise(Net::IMAP::DataFormatError) do From b923dc94931e55cfa03a352c2f0e3ce1e8f6d9b5 Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 4 May 2026 22:40:09 -0400 Subject: [PATCH 03/20] =?UTF-8?q?=F0=9F=8D=92=20pick=203fe06a4c=20(#676):?= =?UTF-8?q?=20=F0=9F=A5=85=20Ensure=20send=5Fnumber=5Fdata=20input=20is=20?= =?UTF-8?q?an=20Integer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/net/imap/command_data.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/net/imap/command_data.rb b/lib/net/imap/command_data.rb index c674f168..cd65e476 100644 --- a/lib/net/imap/command_data.rb +++ b/lib/net/imap/command_data.rb @@ -110,8 +110,9 @@ def send_literal(str, tag = nil, binary: false, non_sync: nil) end end + # NOTE: +num+ should already be an Integer def send_number_data(num) - put_string(num.to_s) + put_string(Integer(num).to_s) end def send_list_data(list, tag = nil) From c2dafab806cb5660157decb8cbfde371443d770a Mon Sep 17 00:00:00 2001 From: nick evans Date: Wed, 15 Apr 2026 18:31:51 -0400 Subject: [PATCH 04/20] =?UTF-8?q?=F0=9F=8D=92=20pick=200d508fa7=20(#681):?= =?UTF-8?q?=20=F0=9F=A5=85=20Validate=20server's=20literal=20byte=20size?= =?UTF-8?q?=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! ---- 馃崚 pick note: `NumValidator` in v0.5 doesn't have `coerce_number64`. Rather than backport _that_, I opted to simply copy a simplified version into both `ResponseReader` and `ResponseParser`. --- lib/net/imap/response_parser.rb | 32 ++++++++++++------- lib/net/imap/response_reader.rb | 15 ++++++++- .../response_parser/quirky_behaviors.yml | 16 ++++++++++ test/net/imap/test_response_reader.rb | 21 ++++++++++++ 4 files changed, 71 insertions(+), 13 deletions(-) diff --git a/lib/net/imap/response_parser.rb b/lib/net/imap/response_parser.rb index 51ba119d..df38e2e4 100644 --- a/lib/net/imap/response_parser.rb +++ b/lib/net/imap/response_parser.rb @@ -2055,10 +2055,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) @@ -2086,10 +2083,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 @@ -2115,10 +2109,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 @@ -2133,6 +2124,23 @@ 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 = coerce_number64 len.to_i + val = @str[@pos, len] + @pos += len + Token.new(type, val) + end + + # copied/adapted from NumValidator in v0.6 + def coerce_number64(num) + int = num.to_i + return int if 0 <= int && int <= 0x7fff_ffff_ffff_ffff + raise DataFormatError, + "number64 must be unsigned 63-bit integer: #{num}" end end diff --git a/lib/net/imap/response_reader.rb b/lib/net/imap/response_reader.rb index 56958eba..ce22abe1 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) @@ -35,7 +37,10 @@ def done?; line_done? && !literal_size end def line_done?; buff.end_with?(CRLF) end 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 @@ -76,6 +81,14 @@ def max_response_remaining! ) end + # copied/adapted from NumValidator in v0.6 + def coerce_number64(num) + int = num.to_i + return int if 0 <= int && int <= 0x7fff_ffff_ffff_ffff + raise DataFormatError, + "number64 must be unsigned 63-bit integer: #{num}" + end + end end end diff --git a/test/net/imap/fixtures/response_parser/quirky_behaviors.yml b/test/net/imap/fixtures/response_parser/quirky_behaviors.yml index e6bf9d48..a0145b60 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 e3e21e7a..e1be671f 100644 --- a/test/net/imap/test_response_reader.rb +++ b/test/net/imap/test_response_reader.rb @@ -28,6 +28,8 @@ def literal(str) "{#{str.bytesize}}\r\n#{str}" end 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, @@ -36,6 +38,8 @@ def literal(str) "{#{str.bytesize}}\r\n#{str}" end zero_literal, illegal_crs, illegal_lfs, + zero_padded, + goofy_zero, simple, ].join) rcvr = Net::IMAP::ResponseReader.new(client, io) @@ -46,6 +50,8 @@ def literal(str) "{#{str.bytesize}}\r\n#{str}" end 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 @@ -85,4 +91,19 @@ def literal(str) "{#{str.bytesize}}\r\n#{str}" end 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 + end From e28915907a98db088a3fd0341015534ae6ba12c3 Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 11 May 2026 11:50:33 -0400 Subject: [PATCH 05/20] =?UTF-8?q?=F0=9F=8D=92=20pick=20b02182ce=20(#678):?= =?UTF-8?q?=20=E2=9C=85=F0=9F=90=9B=20Fix=20FakeServer=20CommandParseError?= =?UTF-8?q?=20(tests=20only)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/net/imap/fake_server/command_reader.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/net/imap/fake_server/command_reader.rb b/test/net/imap/fake_server/command_reader.rb index 03d1da50..64410f42 100644 --- a/test/net/imap/fake_server/command_reader.rb +++ b/test/net/imap/fake_server/command_reader.rb @@ -3,7 +3,7 @@ require "net/imap" class Net::IMAP::FakeServer - CommandParseError = RuntimeError + CommandParseError = Class.new(RuntimeError) class CommandReader attr_reader :last_command @@ -46,7 +46,7 @@ def get_command # TODO: convert bad command exception to tagged BAD response, when possible def parse(buf) /\A([^ ]+) ((?:UID )?\w+)(?: (.+))?\r\n\z/min =~ buf or - raise CommandParseError, "bad request: %p" [buf] + raise CommandParseError, "bad request: %p" % [buf] case $2.upcase when "LOGIN", "SELECT", "EXAMINE", "ENABLE", "AUTHENTICATE" Command.new $1, $2, scan_astrings($3), buf From 8acb3051cb260fe7aec307b6fc23f14c85236b6c Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 11 May 2026 11:52:27 -0400 Subject: [PATCH 06/20] =?UTF-8?q?=F0=9F=8D=92=20pick=20ef6fde3b=20(#678):?= =?UTF-8?q?=20=E2=9C=85=20Handle=20"stream=20closed"=20as=20EOF=20(tests?= =?UTF-8?q?=20only)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/net/imap/fake_server/socket.rb | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/test/net/imap/fake_server/socket.rb b/test/net/imap/fake_server/socket.rb index 65593d86..84e94d58 100644 --- a/test/net/imap/fake_server/socket.rb +++ b/test/net/imap/fake_server/socket.rb @@ -18,10 +18,10 @@ def initialize(tcp_socket, config:) def tls?; !!@tls_socket end def closed?; @closed end - def eof?; socket.eof? end - def gets(...) socket.gets(...) end - def read(...) socket.read(...) end - def print(...) socket.print(...) end + def eof?; ignore_closed?(true) { socket.eof? } end + def gets(...) ignore_closed?(nil) { socket.gets(...) } end + def read(...) ignore_closed?(nil) { socket.read(...) } end + def print(...) ignore_closed?(nil) { socket.print(...) } end def use_tls @tls_socket ||= OpenSSL::SSL::SSLSocket.new(tcp_socket, ssl_ctx).tap do |s| @@ -48,5 +48,13 @@ def ssl_ctx end end + def ignore_closed?(fallback) + yield + rescue IOError => err + close if !closed? && (@tcp_socket.closed? || @tls_socket.closed?) + return fallback if err.message.match?(/stream closed|closed stream/i) + raise + end + end end From 445e596344afa7128201d6424ad47c44b01788a5 Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 11 May 2026 11:56:11 -0400 Subject: [PATCH 07/20] =?UTF-8?q?=F0=9F=8D=92=20pick=20a27a0022=20(#678):?= =?UTF-8?q?=20=E2=9C=85=20Allow=20test=20server=20td=20ignore=20abrupt=20E?= =?UTF-8?q?OF?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This lets us test some specific error conditions, without failing because the server is upset. --- test/net/imap/fake_server/command_reader.rb | 11 ++++++++--- test/net/imap/fake_server/configuration.rb | 3 +++ test/net/imap/fake_server/connection.rb | 2 +- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/test/net/imap/fake_server/command_reader.rb b/test/net/imap/fake_server/command_reader.rb index 64410f42..8857cd9b 100644 --- a/test/net/imap/fake_server/command_reader.rb +++ b/test/net/imap/fake_server/command_reader.rb @@ -6,10 +6,12 @@ class Net::IMAP::FakeServer CommandParseError = Class.new(RuntimeError) class CommandReader + attr_reader :config attr_reader :last_command attr_accessor :literal_acceptor - def initialize(socket) + def initialize(socket, config:) + @config = config @socket = socket @last_command = nil @literal_acceptor = proc {|buff, size| true } @@ -35,8 +37,11 @@ def get_command end throw :eof if buf.empty? @last_command = parse(buf) - rescue CommandParseError => err - raise IOError, err.message if socket.eof? && !buf.end_with?("\r\n") + rescue CommandParseError + if config.ignore_abrupt_eof? && socket.eof? && !buf.end_with?("\r\n") + throw :eof + end + raise end private diff --git a/test/net/imap/fake_server/configuration.rb b/test/net/imap/fake_server/configuration.rb index 91fe72a4..fde14aa7 100644 --- a/test/net/imap/fake_server/configuration.rb +++ b/test/net/imap/fake_server/configuration.rb @@ -45,6 +45,8 @@ class Configuration mailboxes: { "INBOX" => { name: "INBOX" }.freeze, }.freeze, + + ignore_abrupt_eof: false, } def initialize(with_extensions: [], without_extensions: [], **opts, &block) @@ -68,6 +70,7 @@ def initialize(with_extensions: [], without_extensions: [], **opts, &block) alias greeting_bye? greeting_bye alias greeting_capabilities? greeting_capabilities alias sasl_ir? sasl_ir + alias ignore_abrupt_eof? ignore_abrupt_eof def on(event, &handler) handler or raise ArgumentError diff --git a/test/net/imap/fake_server/connection.rb b/test/net/imap/fake_server/connection.rb index 4317364d..e45e6c68 100644 --- a/test/net/imap/fake_server/connection.rb +++ b/test/net/imap/fake_server/connection.rb @@ -12,7 +12,7 @@ def initialize(server, tcp_socket:) @config = server.config @socket = Socket.new tcp_socket, config: config @state = ConnectionState.new socket: socket, config: config - @reader = CommandReader.new socket + @reader = CommandReader.new socket, config: config @writer = ResponseWriter.new socket, config: config, state: state @router = CommandRouter.new writer, config: config, state: state end From ac2c22884ffd558a67feade3486bbec211598fcb Mon Sep 17 00:00:00 2001 From: nick evans Date: Fri, 1 May 2026 14:41:33 -0400 Subject: [PATCH 08/20] =?UTF-8?q?=F0=9F=8D=92=20pick=2095afda8f=20(#679):?= =?UTF-8?q?=20=E2=99=BB=EF=B8=8F=20Allow=20RawData.new=20to=20directly=20s?= =?UTF-8?q?et=20parts=20array?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's convenient that `RawData` can take a string, even though it's composed of parts. But it's surprising, IMO, to _require_ the `data` parameter be a string, when the `data` member is an array. --- lib/net/imap/command_data.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/net/imap/command_data.rb b/lib/net/imap/command_data.rb index cd65e476..284a2962 100644 --- a/lib/net/imap/command_data.rb +++ b/lib/net/imap/command_data.rb @@ -209,7 +209,16 @@ def send_data(imap, tag) imap.__send__(:put_string, data) end class RawData < CommandData # :nodoc: def initialize(data:) - data = split_parts(data) + case data + when String + data = split_parts(data) + when Array + unless data.all? { |part| RawText === part || Literal === part } + raise TypeError, "expected String or Array[#{RawText} | #{Literal}]" + end + else + raise TypeError, "expected String or Array[#{RawText} | #{Literal}]" + end super validate end From 8a737398190af22e05a529dc1e25c1aa1feb2c54 Mon Sep 17 00:00:00 2001 From: nick evans Date: Fri, 1 May 2026 18:07:31 -0400 Subject: [PATCH 09/20] =?UTF-8?q?=F0=9F=8D=92=20pick=20257e51d0=20(#679):?= =?UTF-8?q?=20=E2=99=BB=EF=B8=8F=20Extract=20RawData.split(string)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/net/imap/command_data.rb | 13 ++++++++----- test/net/imap/test_command_data.rb | 17 +++++++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/lib/net/imap/command_data.rb b/lib/net/imap/command_data.rb index 284a2962..22d76c7b 100644 --- a/lib/net/imap/command_data.rb +++ b/lib/net/imap/command_data.rb @@ -211,7 +211,7 @@ class RawData < CommandData # :nodoc: def initialize(data:) case data when String - data = split_parts(data) + data = self.class.split(data) when Array unless data.all? { |part| RawText === part || Literal === part } raise TypeError, "expected String or Array[#{RawText} | #{Literal}]" @@ -233,9 +233,11 @@ def validate end end - private - - def split_parts(data) + # Splits an input +string+ into an array of RawText and Literal/Literal8. + # + # NOTE: unlike RawData#validate, this does not prevent the final RawText + # from ending with a literal prefix. + def self.split(data) data = data.b # dups and ensures BINARY encoding parts = [] while data.match(/(~)?\{(0|[1-9]\d*)(\+)?\}\r\n/n) @@ -252,7 +254,7 @@ def split_parts(data) parts end - def extract_literal(data, binary:, bytesize:, non_sync:) + def self.extract_literal(data, binary:, bytesize:, non_sync:) if data.bytesize < bytesize raise DataFormatError, "Too few bytes in string for literal, " \ "expected: %s, remaining: %s" % [bytesize, data.bytesize] @@ -260,6 +262,7 @@ def extract_literal(data, binary:, bytesize:, non_sync:) literal = data.byteslice(0, bytesize) (binary ? Literal8 : Literal).new(data: literal, non_sync: non_sync) end + private_class_method :extract_literal end class Atom < CommandData # :nodoc: diff --git a/test/net/imap/test_command_data.rb b/test/net/imap/test_command_data.rb index d6c193fe..3801cfeb 100644 --- a/test/net/imap/test_command_data.rb +++ b/test/net/imap/test_command_data.rb @@ -380,6 +380,23 @@ class RawDataTest < CommandDataTest raw = RawData.new(data: " {123} ") assert_equal [RawText[" {123} "]], raw.data end + + data( + "simple raw text" => 'hello "world"', + "text, literal, text" => "OK {5}\r\nhello {5}\r\nworld", + "empty literals" => "{0}\r\n{0+}\r\n~{0}\r\n~{0+}\r\n", + "binary and regular" => "foo ~{7}\r\n\0bar\r\nbaz {4}\r\nquux", + ) + test ".split" do |string| + assert_equal(RawData[string].data, RawData.split(string)) + end + + test ".split allows final literal prefix" do + assert_equal [RawText["text {123}"]], RawData.split("text {123}") + assert_equal [RawText["text+ {123+}"]], RawData.split("text+ {123+}") + assert_equal [RawText["~text ~{123}"]], RawData.split("~text ~{123}") + assert_equal [RawText["~text+ ~{123+}"]], RawData.split("~text+ ~{123+}") + end end end From 0bc84adad0cdfd6346aea26174eb86d8dd900f78 Mon Sep 17 00:00:00 2001 From: nick evans Date: Thu, 4 Jun 2026 09:09:38 -0400 Subject: [PATCH 10/20] =?UTF-8?q?=F0=9F=8D=92=20pick=20aab64f92=20(#686):?= =?UTF-8?q?=20=F0=9F=A7=B5=20Fix=20deadlock=20in=20`#disconnect`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `#disconnect` could deadlock when called inside the mutex, because the receiver thread could be signaling a condition variable and it would thus be unable to resume until the current thread releases its lock. Also, `#disconnect` shouldn't raise `IO#shutdown` exceptions on the receiver thread prior to closing the socket, when it is being called from the receiver thread. The exception is still raised, _after_ the socket is closed. --- lib/net/imap.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 16b91c36..4b594d24 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -1052,6 +1052,7 @@ def client_thread # :nodoc: # Related: #logout, #logout! def disconnect return if disconnected? + in_receiver_thread = Thread.current == @receiver_thread begin begin # try to call SSL::SSLSocket#io. @@ -1063,9 +1064,9 @@ def disconnect rescue Errno::ENOTCONN # ignore `Errno::ENOTCONN: Socket is not connected' on some platforms. rescue Exception => e - @receiver_thread.raise(e) + @receiver_thread.raise(e) unless in_receiver_thread end - @receiver_thread.join + @receiver_thread.join unless mon_owned? || in_receiver_thread synchronize do @sock.close end From e97dcf3f3fa51e22d8e002c804208a4db2e1ac66 Mon Sep 17 00:00:00 2001 From: nick evans Date: Sat, 2 May 2026 14:03:30 -0400 Subject: [PATCH 11/20] =?UTF-8?q?=F0=9F=8D=92=20pick=20e3c50fad=20(#698):?= =?UTF-8?q?=20=E2=99=BB=EF=B8=8F=20Refactor=20RawText,=20add=20improve=20t?= =?UTF-8?q?est=20coverage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also update internal RawText docs. --- lib/net/imap/command_data.rb | 32 ++++++++------- test/net/imap/test_command_data.rb | 63 +++++++++++++++++++++++++----- 2 files changed, 70 insertions(+), 25 deletions(-) diff --git a/lib/net/imap/command_data.rb b/lib/net/imap/command_data.rb index 22d76c7b..4611e400 100644 --- a/lib/net/imap/command_data.rb +++ b/lib/net/imap/command_data.rb @@ -167,36 +167,38 @@ def validate end end - # Represents IMAP +text+ data, which may contain any 7-bit ASCII character, - # except for +NULL+, +CR+, or +LF+. +text+ is extended to allow any - # multibyte +UTF-8+ character when either +UTF8=ACCEPT+ or +IMAP4rev2+ have - # been enabled, or when the server supports only +IMAP4rev2+ and not earlier - # IMAP revisions, or when the server advertises +UTF8=ONLY+. + # Represents IMAP +text+ data, which covers everything in the IMAP grammar, + # except for +literal+, +literal8+, and the concluding +CRLF+. # - # NOTE: The current implementation does not validate whether the connection - # currently supports UTF-8. Future versions may change. + # +data+ may contain any 7-bit ASCII character except +NULL+, +CR+, or +LF+. + # Any multibyte +UTF-8+ character is also allowed when the connection + # supports UTF8: either +UTF8=ACCEPT+ or +IMAP4rev2+ have been enabled, or + # the server supports only +IMAP4rev2+ and not earlier IMAP revisions, or + # the server advertises +UTF8=ONLY+. + # + # NOTE: This does not verify whether the connection supports UTF-8, but that + # may change in future versions. # # The string's bytes must be valid ASCII or valid UTF-8. The string's # reported encoding is ignored, but the string is _not_ transcoded. class RawText < CommandData # :nodoc: def initialize(data:) data = String(data.to_str) - data = if [Encoding::ASCII, Encoding::UTF_8].include?(data.encoding) - -data - elsif data.ascii_only? - -(data.dup.force_encoding("ASCII")) - else - -(data.dup.force_encoding("UTF-8")) + unless [Encoding::ASCII, Encoding::UTF_8].include?(data.encoding) + data = data.dup.force_encoding(data.ascii_only? ? "ASCII" : "UTF-8") end + data = -data super validate end def validate - if data.include?("\0") - raise DataFormatError, "NULL byte must be binary literal encoded" + if ![Encoding::ASCII, Encoding::UTF_8].include?(data.encoding) + raise DataFormatError, "must use ASCII or UTF-8 encoding" elsif !data.valid_encoding? raise DataFormatError, "invalid UTF-8 must be literal encoded" + elsif data.include?("\0") + raise DataFormatError, "NULL byte must be binary literal encoded" elsif /[\r\n]/.match?(data) raise DataFormatError, "CR and LF bytes must be literal encoded" end diff --git a/test/net/imap/test_command_data.rb b/test/net/imap/test_command_data.rb index 3801cfeb..c4fd039c 100644 --- a/test/net/imap/test_command_data.rb +++ b/test/net/imap/test_command_data.rb @@ -180,19 +180,53 @@ def send_data(*data, tag: TAG) end class RawTextTest < CommandDataTest - test "basic ASCII string" do - imap.send_data RawText.new('foo "bar" (baz)') - assert_equal [Output.put_string('foo "bar" (baz)')], imap.output + test "allows ASCII strings with no specials" do + imap.send_data( + RawText["INBOX"], + RawText["etc"] + ) + assert_equal [ + Output.put_string("INBOX"), + Output.put_string("etc"), + ], imap.output + imap.clear end - test "allows IMAP atom-special symbols" do - imap.send_data RawText.new('foo "bar" (baz)') - imap.send_data RawText.new("(){}[]%*\"\\") - imap.send_data RawText.new("(((((((((((((((( unbalanced ]]]]]]]]]]]]]") + test "allows atom specials" do + [ + " with spaces in string ", + "with_parens()", + "with_list_wildcards*", + "with_list_wildcards%", + "with_resp_special]", + "with\x7fcontrol_char", + %{(){}[]%*'}, + ].each do |string| + imap.send_data RawText[string] + end assert_equal [ - Output.put_string('foo "bar" (baz)'), - Output.put_string("(){}[]%*\"\\"), - Output.put_string("(((((((((((((((( unbalanced ]]]]]]]]]]]]]"), + Output.put_string(" with spaces in string "), + Output.put_string("with_parens()"), + Output.put_string("with_list_wildcards*"), + Output.put_string("with_list_wildcards%"), + Output.put_string("with_resp_special]"), + Output.put_string("with\x7fcontrol_char"), + Output.put_string(%{(){}[]%*'}), + ], imap.output + end + + test "allows quoted specials" do + [ + '"with" "quoted" "specials"', + '\\with\\quoted\\specials\\', + %{(){}[]%*"'\\}, + ].each do |string| + imap.send_data RawText[string] + end + assert_equal [ + Output.put_string('"with" "quoted" "specials"'), + Output.put_string('\\with\\quoted\\specials\\'), + Output.put_string(%{(){}[]%*"'\\}), ], imap.output end @@ -211,6 +245,15 @@ class RawTextTest < CommandDataTest ], imap.output end + test "allows valid UTF-8 multibyte chars" do + imap.send_data RawText.new("f枚贸 b盲r") + imap.send_data RawText.new("銇汇亽 銇点亴 銇淬倛") + assert_equal [ + Output.put_string("f枚贸 b盲r"), + Output.put_string("銇汇亽 銇点亴 銇淬倛"), + ], imap.output + end + data( "NULL" => ["with \0 NULL", /NULL\b.+\bbyte/i], "CR" => ["with \r CR", /CR\b.+\bbyte/i], From 2ccfaaee92be1daed44766c48c2f220d68783f38 Mon Sep 17 00:00:00 2001 From: nick evans Date: Sat, 2 May 2026 14:02:18 -0400 Subject: [PATCH 12/20] =?UTF-8?q?=F0=9F=8D=92=20pick=208d9397ab=20(#698):?= =?UTF-8?q?=20=F0=9F=A5=85=20Validate=20QuotedString=20contains=20only=20v?= =?UTF-8?q?alid=20bytes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This shares most of the validation implementation with RawText, via an extracted shared superclass. Please note: in currently released `net-imap`, QuotedString is _not_ used to quote regular string arguments. It's currently only used by `Net::IMAP#id` (the `ID` extension). Without this patch, `Net::IMAP#id` is vulnerable to the same CRLF injection issues in GHSA-75xq-5h9v-w6px and GHSA-hm49-wcqc-g2xg. The string will be quoted, which may limit the ability to inject some commands. Presumably, `Net::IMAP#id` is unlikely to be called with user-provided input, making this less likely to be exploitable. Nevertheless, CRLF injection should be prevented! --- lib/net/imap/command_data.rb | 38 ++++++----- test/net/imap/test_command_data.rb | 102 ++++++++++++++++++++++++++--- 2 files changed, 113 insertions(+), 27 deletions(-) diff --git a/lib/net/imap/command_data.rb b/lib/net/imap/command_data.rb index 4611e400..4e00d814 100644 --- a/lib/net/imap/command_data.rb +++ b/lib/net/imap/command_data.rb @@ -167,8 +167,8 @@ def validate end end - # Represents IMAP +text+ data, which covers everything in the IMAP grammar, - # except for +literal+, +literal8+, and the concluding +CRLF+. + # Represents IMAP +text+ or +quoted+ data, which share the same + # validations of decoded #data, and differ only in how they are formatted. # # +data+ may contain any 7-bit ASCII character except +NULL+, +CR+, or +LF+. # Any multibyte +UTF-8+ character is also allowed when the connection @@ -181,7 +181,7 @@ def validate # # The string's bytes must be valid ASCII or valid UTF-8. The string's # reported encoding is ignored, but the string is _not_ transcoded. - class RawText < CommandData # :nodoc: + class ValidNonLiteralData < CommandData def initialize(data:) data = String(data.to_str) unless [Encoding::ASCII, Encoding::UTF_8].include?(data.encoding) @@ -206,7 +206,17 @@ def validate def ascii_only?; data.ascii_only? end - def send_data(imap, tag) imap.__send__(:put_string, data) end + def send_data(imap, tag = nil) imap.__send__(:put_string, formatted) end + end + + # Represents IMAP +text+ data, which covers everything in the IMAP grammar, + # except for +literal+, +literal8+, and the concluding +CRLF+. + # + # NOTE: The current implementation does not verify that the connection + # supports UTF-8. Future versions may validate this. + class RawText < ValidNonLiteralData # :nodoc: + # raw: no formatting necessary + alias formatted data end class RawData < CommandData # :nodoc: @@ -293,19 +303,13 @@ def send_data(imap, tag) end end - class QuotedString # :nodoc: - def send_data(imap, tag) - imap.__send__(:send_quoted_string, @data) - end - - def validate - end - - private - - def initialize(data) - @data = data - end + # Represents a IMAP +quoted+ string, which can encode any valid ASCII or + # UTF-8 string, unless it contains any +CR+, +LF+, or +NULL+ bytes. + # + # NOTE: The current implementation does not verify that the connection + # supports UTF-8. Future versions may validate this. + class QuotedString < ValidNonLiteralData # :nodoc: + def formatted; %("#{data.gsub(/["\\]/, "\\\\\\&")}") end end class Literal # :nodoc: diff --git a/test/net/imap/test_command_data.rb b/test/net/imap/test_command_data.rb index c4fd039c..6ba7e9d5 100644 --- a/test/net/imap/test_command_data.rb +++ b/test/net/imap/test_command_data.rb @@ -8,6 +8,7 @@ class CommandDataTest < Test::Unit::TestCase Atom = Net::IMAP::Atom Flag = Net::IMAP::Flag + QuotedString = Net::IMAP::QuotedString Literal = Net::IMAP::Literal Literal8 = Net::IMAP::Literal8 RawText = Net::IMAP::RawText @@ -179,6 +180,83 @@ def send_data(*data, tag: TAG) ], imap.output end + class QuotedStringTest < CommandDataTest + test "quotes ASCII strings (no specials)" do + assert_equal '"INBOX"', QuotedString["INBOX"].formatted + imap.send_data( + QuotedString["INBOX"], + QuotedString["etc"] + ) + assert_equal [ + Output.put_string('"INBOX"'), + Output.put_string('"etc"'), + ], imap.output + imap.clear + end + + test "quotes ASCII strings (atom specials)" do + [ + " with spaces in string ", + "with_parens()", + "with_list_wildcards*", + "with_list_wildcards%", + "with_resp_special]", + "with\x7fcontrol_char", + %{(){}[]%*'}, + ].each do |string| + imap.send_data QuotedString[string] + end + assert_equal [ + Output.put_string('" with spaces in string "'), + Output.put_string('"with_parens()"'), + Output.put_string('"with_list_wildcards*"'), + Output.put_string('"with_list_wildcards%"'), + Output.put_string('"with_resp_special]"'), + Output.put_string(%{"with\x7fcontrol_char"}), + Output.put_string(%Q{"(){}[]%*'"}), + ], imap.output + end + + test "escapes quoted specials" do + [ + '"with" "quoted" "specials"', + "\\with\\quoted\\specials\\", + %{(){}[]%*"'\\}, + ].each do |string| + imap.send_data QuotedString[string] + end + assert_equal [ + Output.put_string('"\"with\" \"quoted\" \"specials\""'), + Output.put_string('"\\\\with\\\\quoted\\\\specials\\\\"'), + Output.put_string(%q{"(){}[]%*\"'\\\\"}), + ], imap.output + end + + test "ASCII compatible string with another encodings" do + imap.send_data QuotedString.new("foo bar".encode("cp1252")) + assert_equal [ + Output.put_string('"foo bar"'), + ], imap.output + end + + test "allows ASCII control chars" do + text = QuotedString.new("beep\b beep\b escape!\e delete this:\x1f") + imap.send_data text + assert_equal [ + Output.put_string(%{"beep\b beep\b escape!\e delete this:\x1f"}), + ], imap.output + end + + test "quotes valid UTF-8 multibyte chars" do + imap.send_data QuotedString.new("f枚贸 b盲r") + imap.send_data QuotedString.new("銇汇亽 銇点亴 銇淬倛") + assert_equal [ + Output.put_string('"f枚贸 b盲r"'), + Output.put_string('"銇汇亽 銇点亴 銇淬倛"'), + ], imap.output + end + end + class RawTextTest < CommandDataTest test "allows ASCII strings with no specials" do imap.send_data( @@ -253,14 +331,20 @@ class RawTextTest < CommandDataTest Output.put_string("銇汇亽 銇点亴 銇淬倛"), ], imap.output end + end + SharedValidNonLiteralDataTests = ->(data_type) do data( "NULL" => ["with \0 NULL", /NULL\b.+\bbyte/i], "CR" => ["with \r CR", /CR\b.+\bbyte/i], "LF" => ["with \n LF", /LF\b.+\bbyte/i], ) test "invalid ASCII byte" do |(text, error_message)| - try_multiple_encodings(error_message, text) + with_multiple_encodings(text) do |encoded| + assert_raise_with_message(DataFormatError, error_message) do + data_type[encoded] + end + end end # See Table 3-7, Well-Formed UTF-8 Byte Sequences, in The Unicode Standard: @@ -277,7 +361,11 @@ class RawTextTest < CommandDataTest "windows-1252" => "氓锚茂玫眉".encode("windows-1252"), ) test "invalid UTF-8" do |text| - try_multiple_encodings(/invalid UTF-8/i, text) + with_multiple_encodings(text) do |encoded| + assert_raise_with_message(DataFormatError, /invalid UTF-8/i) do + data_type[encoded] + end + end end def with_multiple_encodings(data) @@ -286,15 +374,9 @@ def with_multiple_encodings(data) yield data.dup.force_encoding("UTF-8") yield data.dup.force_encoding("cp1252") end - - def try_multiple_encodings(error_message, data) - with_multiple_encodings(data) do |encoded| - assert_raise_with_message(DataFormatError, error_message) do - RawText[encoded] - end - end - end end + QuotedStringTest.class_exec QuotedString, &SharedValidNonLiteralDataTests + RawTextTest .class_exec RawText, &SharedValidNonLiteralDataTests class RawDataTest < CommandDataTest test "simple raw text" do From 3ee25e354c6abb2977736b8445fd636a1ab6461e Mon Sep 17 00:00:00 2001 From: nick evans Date: Fri, 22 May 2026 15:33:55 -0400 Subject: [PATCH 13/20] =?UTF-8?q?=F0=9F=8D=92=20pick=201f97168b=20(#699):?= =?UTF-8?q?=20=F0=9F=A5=85=20Validate=20`#enable`=20arguments=20are=20all?= =?UTF-8?q?=20atoms?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This still allows the argument to be a single string with multiple space-delimited arguments. It splits the string first, and validates the substrings. --- lib/net/imap.rb | 5 +++-- test/net/imap/test_imap.rb | 8 ++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 4b594d24..8c660c06 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -2561,10 +2561,11 @@ def enable(*capabilities) capabilities = capabilities .flatten .map {|e| ENABLE_ALIASES[e] || e } + .flat_map { _1.is_a?(String) && !_1.empty? ? _1.split(/ /, -1) : [_1] } .uniq - .join(' ') + .map { Atom[_1] } synchronize do - send_command("ENABLE #{capabilities}") + send_command("ENABLE", *capabilities) result = clear_responses("ENABLED").last || [] @utf8_strings ||= result.include? "UTF8=ACCEPT" @utf8_strings ||= result.include? "IMAP4REV2" diff --git a/test/net/imap/test_imap.rb b/test/net/imap/test_imap.rb index 678c46b5..89b72dfc 100644 --- a/test/net/imap/test_imap.rb +++ b/test/net/imap/test_imap.rb @@ -1216,6 +1216,14 @@ def test_enable assert_equal %w[CONDSTORE], result1 assert_equal %w[UTF8=ACCEPT], result2 assert_equal [], result3 + + assert_raise(Net::IMAP::DataFormatError) do + imap.enable "injection\r\ninjected logout" + end + assert_empty cmdq + assert_raise(Net::IMAP::DataFormatError) do + imap.enable "foo", "", "bar" + end end end From 90b334c0396b8428c41274c5c66fca681e23153a Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 11 May 2026 08:51:25 -0400 Subject: [PATCH 14/20] =?UTF-8?q?=F0=9F=8D=92=20pick=20d6ddd294=20(#700):?= =?UTF-8?q?=20=F0=9F=90=9B=20Prevent=20trailing=20`{0}`=20in=20RawData=20v?= =?UTF-8?q?alidation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zero-length literals are explicitly allowed by the RFCs and this did not catch text that ends with `{0}` or `{0+}`. This leaves RawData able to absorb the `CRLF` that ends the command, and thus absorb the following command into itself. Ultimately, we don't care if the `number64` is encoded correctly nor whether it claims to be a binary literal. So I've simplified the regexp by dropping `~?` and using `\d+` for the number. --- lib/net/imap/command_data.rb | 2 +- test/net/imap/test_command_data.rb | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/net/imap/command_data.rb b/lib/net/imap/command_data.rb index 4e00d814..1e017d46 100644 --- a/lib/net/imap/command_data.rb +++ b/lib/net/imap/command_data.rb @@ -240,7 +240,7 @@ def send_data(imap, tag) data.each do _1.send_data(imap, tag) end end def validate return unless RawText === data.last text = data.last.data - if text.rindex(/~?\{[1-9]\d*\+?\}\z/n) + if text.rindex(/\{\d+\+?\}\z/n) raise DataFormatError, "RawData cannot end with literal continuation" end end diff --git a/test/net/imap/test_command_data.rb b/test/net/imap/test_command_data.rb index 6ba7e9d5..bb460cd8 100644 --- a/test/net/imap/test_command_data.rb +++ b/test/net/imap/test_command_data.rb @@ -504,6 +504,13 @@ class RawDataTest < CommandDataTest assert_raise(DataFormatError) do RawData.new(data: "~literal+ ~{123+}") end raw = RawData.new(data: " {123} ") assert_equal [RawText[" {123} "]], raw.data + + assert_raise(DataFormatError) do RawData.new(data: "literal {0}") end + assert_raise(DataFormatError) do RawData.new(data: "literal+ {0+}") end + assert_raise(DataFormatError) do RawData.new(data: "~literal ~{0}") end + assert_raise(DataFormatError) do RawData.new(data: "~literal+ ~{0+}") end + raw = RawData.new(data: " {0} ") + assert_equal [RawText[" {0} "]], raw.data end data( From 33348a4bcdf83a71b4f72c14f8b8f97f9aa11188 Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 11 May 2026 11:59:08 -0400 Subject: [PATCH 15/20] =?UTF-8?q?=F0=9F=8D=92=20pick=2062a0da6d=20(#701):?= =?UTF-8?q?=20=F0=9F=A5=85=20Validate=20non-synchronizing=20literals=20sup?= =?UTF-8?q?port?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's bad behavior to send a non-synchronizing literal that's too large. This forces the server to choose between reading but ignoring the bytes or closing the connection. But, for servers that _don't_ support non-synchronizing literals, this could be another CRLF/command injection attack vector. If a server sees the `}\r\n` but can't parse the literal bytesize, it may decide to close the connection, and all is fine. But, a server _might_ respond to any unparseable command line (ending in `CRLF`) with `BAD`, then interpret the literal as the next command. In that case, a CRLF/command injection could succeed. Fortunately, `LITERAL-` is supported by most IMAP servers. So this is not expected to be widely exploitable. --- lib/net/imap/command_data.rb | 18 ++++++++++++- test/net/imap/test_imap.rb | 52 +++++++++++++++++++++++++++++++++++- 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/lib/net/imap/command_data.rb b/lib/net/imap/command_data.rb index 1e017d46..baaaf1ee 100644 --- a/lib/net/imap/command_data.rb +++ b/lib/net/imap/command_data.rb @@ -83,12 +83,19 @@ def send_binary_literal(*a, **kw) send_literal(*a, **kw, binary: true) end # `non_sync` is an optional tri-state flag: # * `true` -> Force non-synchronizing `LITERAL+`/`LITERAL-` behavior. - # TODO: raise or warn when capabilities don't allow non_sync. + # NOTE: raises DataFormatError when server doesn't support + # non-synchronizing literal, or literal is too large for LITERAL-. # * `false` -> Force normal synchronizing literal behavior. # * `nil` -> (default) Currently behaves like `false` (will be dynamic). # TODO: Dynamic, based on capabilities and bytesize. def send_literal(str, tag = nil, binary: false, non_sync: nil) synchronize do + if non_sync && !non_sync_literal_allowed?(str.bytesize) + # TODO: check in Printer, so we don't need to close the connection. + @sock.close + raise DataFormatError, "Connection closed: " \ + "Cannot send non-synchronizing literal without known server support" + end prefix = "~" if binary plus = "+" if non_sync put_string("#{prefix}{#{str.bytesize}#{plus}}\r\n") @@ -110,6 +117,15 @@ def send_literal(str, tag = nil, binary: false, non_sync: nil) end end + def non_sync_literal_allowed?(bytesize) + return unless capabilities_cached? + return "+" if capable?("LITERAL+") + return "-" if capable_literal_minus? && bytesize <= 4096 + false + end + + def capable_literal_minus?; capable?("LITERAL-") || capable?("IMAP4rev2") end + # NOTE: +num+ should already be an Integer def send_number_data(num) put_string(Integer(num).to_s) diff --git a/test/net/imap/test_imap.rb b/test/net/imap/test_imap.rb index 89b72dfc..ca885beb 100644 --- a/test/net/imap/test_imap.rb +++ b/test/net/imap/test_imap.rb @@ -753,7 +753,7 @@ def test_raw_data end test("send literal args") do - with_fake_server do |server, imap| + with_fake_server(with_extensions: %w[LITERAL-]) do |server, imap| server.on "TEST", &:done_ok send_args = ->(*args) do imap.__send__(:send_command, "TEST", *args) @@ -802,6 +802,56 @@ def test_raw_data end end + test("send non-synchronizing literals with LITERAL+") do + with_fake_server( + with_extensions: %w[LITERAL+], greeting_capabilities: true, + ) do |server, imap| + def imap.send_test_args(*args) = send_command("TEST", *args) + server.on "TEST", &:done_ok + + # imap.config.max_non_synchronizing_literal = 5_000 + # NOTE: support for automatic non-synchronizing literals added in v0.6 + large = "\xff".b * 5_000 + imap.send_test_args Net::IMAP::Literal[large, nil] + assert_equal("{5000}\r\n#{large}".b, server.commands.pop.args) + + large = "\xff".b * 10_000 + imap.send_test_args Net::IMAP::Literal[large, nil] + assert_equal("{10000}\r\n#{large}".b, server.commands.pop.args) + + imap.send_test_args Net::IMAP::Literal[large, true] + assert_equal("{10000+}\r\n#{large}".b, server.commands.pop.args) + end + end + + test("send non-synchronizing literal that's too large for LITERAL-") do + with_fake_server( + with_extensions: %w[LITERAL-], greeting_capabilities: true, + ignore_abrupt_eof: true + ) do |server, imap| + def imap.send_test_args(*args) = send_command("TEST", *args) + server.on "TEST", &:done_ok + assert_raise(Net::IMAP::DataFormatError) do + imap.send_test_args Net::IMAP::Literal["\xff".b * 5000, true] + end + assert imap.disconnected? + end + end + + test("send non-synchronizing literal without known server support") do + with_fake_server( + with_extensions: %w[LITERAL+], greeting_capabilities: false, + ignore_abrupt_eof: true + ) do |server, imap| + def imap.send_test_args(*args) = send_command("TEST", *args) + server.on "TEST", &:done_ok + assert_raise(Net::IMAP::DataFormatError) do + imap.send_test_args Net::IMAP::Literal["\xff".b * 100, true] + end + assert imap.disconnected? + end + end + def test_disconnect server = create_tcp_server port = server.addr[1] From 9d262bd29eda3953ecc4293b5a98701b5a08f1f7 Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 11 May 2026 12:14:25 -0400 Subject: [PATCH 16/20] =?UTF-8?q?=F0=9F=8D=92=20pick=20ae9f83b5=20(#701):?= =?UTF-8?q?=20=E2=99=BB=EF=B8=8F=20Extract=20str.bytesize=20lvar=20in=20se?= =?UTF-8?q?nd=5Fliteral?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/net/imap/command_data.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/net/imap/command_data.rb b/lib/net/imap/command_data.rb index baaaf1ee..77522224 100644 --- a/lib/net/imap/command_data.rb +++ b/lib/net/imap/command_data.rb @@ -89,8 +89,9 @@ def send_binary_literal(*a, **kw) send_literal(*a, **kw, binary: true) end # * `nil` -> (default) Currently behaves like `false` (will be dynamic). # TODO: Dynamic, based on capabilities and bytesize. def send_literal(str, tag = nil, binary: false, non_sync: nil) + bytesize = str.bytesize synchronize do - if non_sync && !non_sync_literal_allowed?(str.bytesize) + if non_sync && !non_sync_literal_allowed?(bytesize) # TODO: check in Printer, so we don't need to close the connection. @sock.close raise DataFormatError, "Connection closed: " \ @@ -98,7 +99,7 @@ def send_literal(str, tag = nil, binary: false, non_sync: nil) end prefix = "~" if binary plus = "+" if non_sync - put_string("#{prefix}{#{str.bytesize}#{plus}}\r\n") + put_string("#{prefix}{#{bytesize}#{plus}}\r\n") if non_sync put_string(str) return From 0343c5802e78b15297be4163d74ecfadb8dcbb82 Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 9 Jun 2026 11:12:36 -0400 Subject: [PATCH 17/20] =?UTF-8?q?=F0=9F=8D=92=20pick=200ea9eba3=20(#701):?= =?UTF-8?q?=20=E2=9C=85=20Fix=20flaky=20tests=20for=20MacOS,=20TruffleRuby?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These tests can raise `Errno::ECONNREST, "Connection reset"` in the server thread. I've only seen it in TruffleRuby (semi-reliably) and MacOS (flaky), but probably it's a timing issue and can happen elsewhere too? --- test/net/imap/test_imap.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/net/imap/test_imap.rb b/test/net/imap/test_imap.rb index ca885beb..f375e466 100644 --- a/test/net/imap/test_imap.rb +++ b/test/net/imap/test_imap.rb @@ -827,7 +827,7 @@ def imap.send_test_args(*args) = send_command("TEST", *args) test("send non-synchronizing literal that's too large for LITERAL-") do with_fake_server( with_extensions: %w[LITERAL-], greeting_capabilities: true, - ignore_abrupt_eof: true + ignore_abrupt_eof: true, ignore_io_error: true ) do |server, imap| def imap.send_test_args(*args) = send_command("TEST", *args) server.on "TEST", &:done_ok @@ -841,7 +841,7 @@ def imap.send_test_args(*args) = send_command("TEST", *args) test("send non-synchronizing literal without known server support") do with_fake_server( with_extensions: %w[LITERAL+], greeting_capabilities: false, - ignore_abrupt_eof: true + ignore_abrupt_eof: true, ignore_io_error: true ) do |server, imap| def imap.send_test_args(*args) = send_command("TEST", *args) server.on "TEST", &:done_ok From 807b0079dce54ebfc10cb8dd7fd21c8469daa21f Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Mon, 15 Jun 2026 15:52:44 +0900 Subject: [PATCH 18/20] =?UTF-8?q?=E2=9C=85=20Avoid=20endless=20method=20de?= =?UTF-8?q?fs=20for=20Ruby=202.7=20compatibility?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The non-synchronizing literal tests from the #701 backport used endless method definitions, which require Ruby 3.0. The v0.4 line still supports Ruby 2.7.3, so this broke CI on 2.7. --- test/net/imap/test_imap.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/net/imap/test_imap.rb b/test/net/imap/test_imap.rb index f375e466..cbbe6bab 100644 --- a/test/net/imap/test_imap.rb +++ b/test/net/imap/test_imap.rb @@ -806,7 +806,7 @@ def test_raw_data with_fake_server( with_extensions: %w[LITERAL+], greeting_capabilities: true, ) do |server, imap| - def imap.send_test_args(*args) = send_command("TEST", *args) + def imap.send_test_args(*args) send_command("TEST", *args) end server.on "TEST", &:done_ok # imap.config.max_non_synchronizing_literal = 5_000 @@ -829,7 +829,7 @@ def imap.send_test_args(*args) = send_command("TEST", *args) with_extensions: %w[LITERAL-], greeting_capabilities: true, ignore_abrupt_eof: true, ignore_io_error: true ) do |server, imap| - def imap.send_test_args(*args) = send_command("TEST", *args) + def imap.send_test_args(*args) send_command("TEST", *args) end server.on "TEST", &:done_ok assert_raise(Net::IMAP::DataFormatError) do imap.send_test_args Net::IMAP::Literal["\xff".b * 5000, true] @@ -843,7 +843,7 @@ def imap.send_test_args(*args) = send_command("TEST", *args) with_extensions: %w[LITERAL+], greeting_capabilities: false, ignore_abrupt_eof: true, ignore_io_error: true ) do |server, imap| - def imap.send_test_args(*args) = send_command("TEST", *args) + def imap.send_test_args(*args) send_command("TEST", *args) end server.on "TEST", &:done_ok assert_raise(Net::IMAP::DataFormatError) do imap.send_test_args Net::IMAP::Literal["\xff".b * 100, true] From f9020baf38f953f48310e0b690818e83ddc2cee2 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Mon, 15 Jun 2026 16:05:23 +0900 Subject: [PATCH 19/20] =?UTF-8?q?=F0=9F=A7=B5=20Synchronize=20FakeServer::?= =?UTF-8?q?Connection#close=20to=20avoid=20double=20logout?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a client closes the socket abruptly, the server thread's `run` ensure and the test helper's `shutdown` can call `close` concurrently. Without a mutex the `state.logout?` check races, so both callers run `state.logout` and the second raises "already logged out", flaking the send_literal tests on ubuntu CI. --- test/net/imap/fake_server/connection.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/test/net/imap/fake_server/connection.rb b/test/net/imap/fake_server/connection.rb index e45e6c68..df428828 100644 --- a/test/net/imap/fake_server/connection.rb +++ b/test/net/imap/fake_server/connection.rb @@ -15,6 +15,7 @@ def initialize(server, tcp_socket:) @reader = CommandReader.new socket, config: config @writer = ResponseWriter.new socket, config: config, state: state @router = CommandRouter.new writer, config: config, state: state + @mutex = Thread::Mutex.new end def commands; state.commands end @@ -34,11 +35,13 @@ def run end def close - unless state.logout? - state.logout - writer.bye + @mutex.synchronize do + unless state.logout? + state.logout + writer.bye + end + socket&.close unless socket&.closed? end - socket&.close unless socket&.closed? end private From 8a238ad8e42662a086011f1f2747296f9b94ec87 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Mon, 15 Jun 2026 14:38:12 +0900 Subject: [PATCH 20/20] =?UTF-8?q?=F0=9F=94=96=20Bump=20version=20to=200.4.?= =?UTF-8?q?25?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/net/imap.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 8c660c06..4abe4481 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -779,7 +779,7 @@ module Net # * {IMAP URLAUTH Authorization Mechanism Registry}[https://www.iana.org/assignments/urlauth-authorization-mechanism-registry/urlauth-authorization-mechanism-registry.xhtml] # class IMAP < Protocol - VERSION = "0.4.24" + VERSION = "0.4.25" # Aliases for supported capabilities, to be used with the #enable command. ENABLE_ALIASES = {