From 62a0da6d1f78610b15941758db4a28f027a25666 Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 11 May 2026 11:59:08 -0400 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=A5=85=20Validate=20non-synchronizing?= =?UTF-8?q?=20literals=20support?= 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 | 24 ++++++++++++++---- test/net/imap/test_imap.rb | 49 ++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/lib/net/imap/command_data.rb b/lib/net/imap/command_data.rb index 9a5749b5..5fd9826a 100644 --- a/lib/net/imap/command_data.rb +++ b/lib/net/imap/command_data.rb @@ -85,11 +85,18 @@ def send_binary_literal(*, **) = send_literal(*, **, binary: true) # `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). 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 non_sync = non_sync_literal?(str.bytesize) if non_sync.nil? prefix = "~" if binary plus = "+" if non_sync @@ -113,12 +120,19 @@ def send_literal(str, tag = nil, binary: false, non_sync: nil) end def non_sync_literal?(bytesize) - capabilities_cached? && - bytesize <= config.max_non_synchronizing_literal && - (capable?("LITERAL+") || - bytesize <= 4096 && (capable?("IMAP4rev2") || capable?("LITERAL-"))) + bytesize <= config.max_non_synchronizing_literal \ + && non_sync_literal_allowed?(bytesize) + 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") + # 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 f43bdcc7..b1df2b75 100644 --- a/test/net/imap/test_imap.rb +++ b/test/net/imap/test_imap.rb @@ -869,6 +869,55 @@ 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 + 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 ae9f83b59cd33265179e5ad406efe830ad246204 Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 11 May 2026 12:14:25 -0400 Subject: [PATCH 2/3] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Extract=20str.bytesize?= =?UTF-8?q?=20lvar=20in=20send=5Fliteral?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/net/imap/command_data.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/net/imap/command_data.rb b/lib/net/imap/command_data.rb index 5fd9826a..d0a5fc51 100644 --- a/lib/net/imap/command_data.rb +++ b/lib/net/imap/command_data.rb @@ -90,17 +90,18 @@ def send_binary_literal(*, **) = send_literal(*, **, binary: true) # * `false` -> Force normal synchronizing literal behavior. # * `nil` -> (default) Currently behaves like `false` (will be dynamic). 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: " \ "Cannot send non-synchronizing literal without known server support" end - non_sync = non_sync_literal?(str.bytesize) if non_sync.nil? + non_sync = non_sync_literal?(bytesize) if non_sync.nil? 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 0ea9eba30b060380efa83980ae6f62c58ffa97e7 Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 9 Jun 2026 11:12:36 -0400 Subject: [PATCH 3/3] =?UTF-8?q?=E2=9C=85=20Fix=20flaky=20tests=20for=20Mac?= =?UTF-8?q?OS,=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 b1df2b75..321cc258 100644 --- a/test/net/imap/test_imap.rb +++ b/test/net/imap/test_imap.rb @@ -893,7 +893,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 @@ -907,7 +907,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