From c67470092e53d5f8d1f8d47c80450dd7b5995302 Mon Sep 17 00:00:00 2001 From: nick evans Date: Wed, 8 Jan 2025 22:06:47 -0500 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=94=92=20Limit=20exponential=20memory?= =?UTF-8?q?=20usage=20to=20parse=20uid-set?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The UID sets in UIDPlusData are stored as arrays of UIDs. In common scenarios, copying between one and a few hundred emails at a time, this is barely noticable. But the memory use expands _exponentially_. This should not be an issue for _trusted_ servers, and (I assume) compromised servers will be more interested in evading detection and stealing your credentials and your email than in causing client Denial of Service. Nevertheless, this is a very simple DoS attack against clients connecting to untrusted servers (for example, a service that connects to user-specified servers). For example, assuming a 64-bit architecture, considering only the data in the two arrays, assuming the arrays' internal capacity is no more than needed, and ignoring the fixed cost of the response structs: * 32 bytes expands to ~160KB (about 5000 times more): `"* OK [COPYUID 1 1:9999 1:9999]\r\n"` * 40 bytes expands to ~1.6GB (about 50 million times more): `"* OK [COPYUID 1 1:99999999 1:99999999]\r\n"` * In the worst scenario (uint32 max), 44 bytes expands to 64GiB in memory, using over 1.5 billion times more to store than to send: `"* OK [COPYUID 1 1:4294967295 1:4294967295]\r\n"` ---- The preferred fix is to store `uid-set` as a SequenceSet, not an array. Unfortunately, this is not fully backwards compatible. For v0.4 and v0.5, use `Config#parser_use_deprecated_uidplus_data` to false to use AppendUIDData and CopyUIDData instead of UIDPlusData. Unless you are _using_ UIDPLUS, this is completely safe. v0.6 will drop UIDPlusData. ---- The simplest _partial_ fix (preserving full backward compatibility) is to raise an error when the number of UIDs goes over some threshold, and continue using arrays inside UIDPlusData. For v0.3.x (and in this commit) the maximum count is hard-coded to 10,000. This is high enough that it should almost never be triggered by normal usage, and low enough to be a less extreme problem. For v0.4 and v0.5, the next commit will make the maximum array size configurable, with a much lower default: 1000 for 0.4 and 100 for 0.5. These are low enough that they are _unlikely_ to cause a problem, but 0.4 and 0.5 can also use the newer AppendUIDData and CopyUIDData classes. However, because unhandled responses are stored on the `#responses` hash, this can still be a problem. A malicious server could repeatedly use 160Kb of client memory by sending only 32 bytes in a loop. To fully solve this problem, a response handler must be added to prune excessive APPENDUID/COPYUID responses as they are received. Because unhandled responses have always been retained, managing unhandled responses is already documented as necessary for long-lived connections. --- lib/net/imap/response_parser.rb | 15 ++++++++++++--- test/net/imap/test_imap_response_parser.rb | 10 ++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/lib/net/imap/response_parser.rb b/lib/net/imap/response_parser.rb index 71c08e7b..826ca527 100644 --- a/lib/net/imap/response_parser.rb +++ b/lib/net/imap/response_parser.rb @@ -8,6 +8,8 @@ class IMAP < Protocol # Parses an \IMAP server response. class ResponseParser + MAX_UID_SET_SIZE = 10_000 + include ParserUtils extend ParserUtils::Generator @@ -2023,9 +2025,16 @@ def CopyUID(...) DeprecatedUIDPlus(...) || CopyUIDData.new(...) end # TODO: remove this code in the v0.6.0 release def DeprecatedUIDPlus(validity, src_uids = nil, dst_uids) return unless config.parser_use_deprecated_uidplus_data - src_uids &&= src_uids.each_ordered_number.to_a - dst_uids = dst_uids.each_ordered_number.to_a - UIDPlusData.new(validity, src_uids, dst_uids) + compact_uid_sets = [src_uids, dst_uids].compact + count = compact_uid_sets.map { _1.count_with_duplicates }.max + max = MAX_UID_SET_SIZE + if count <= max + src_uids &&= src_uids.each_ordered_number.to_a + dst_uids = dst_uids.each_ordered_number.to_a + UIDPlusData.new(validity, src_uids, dst_uids) + else + parse_error("uid-set is too large: %d > %d", count, max) + end end ADDRESS_REGEXP = /\G diff --git a/test/net/imap/test_imap_response_parser.rb b/test/net/imap/test_imap_response_parser.rb index 7a70c02b..33427e6a 100644 --- a/test/net/imap/test_imap_response_parser.rb +++ b/test/net/imap/test_imap_response_parser.rb @@ -215,6 +215,11 @@ def test_fetch_binary_and_binary_size parser = Net::IMAP::ResponseParser.new(config: { parser_use_deprecated_uidplus_data: true, }) + assert_raise_with_message Net::IMAP::ResponseParseError, /uid-set is too large/ do + parser.parse( + "A004 OK [APPENDUID 1 10000:20000,1] Done\r\n" + ) + end response = parser.parse("A004 OK [APPENDUID 1 101:200] Done\r\n") uidplus = response.data.code.data assert_instance_of Net::IMAP::UIDPlusData, uidplus @@ -263,6 +268,11 @@ def test_fetch_binary_and_binary_size parser = Net::IMAP::ResponseParser.new(config: { parser_use_deprecated_uidplus_data: true, }) + assert_raise_with_message Net::IMAP::ResponseParseError, /uid-set is too large/ do + parser.parse( + "A004 OK [copyUID 1 10000:20000,1 1:10001] Done\r\n" + ) + end response = parser.parse("A004 OK [copyUID 1 101:200 1:100] Done\r\n") uidplus = response.data.code.data assert_instance_of Net::IMAP::UIDPlusData, uidplus From 2f58d020580176ed13fcd1e571ab7bc0e1e8f155 Mon Sep 17 00:00:00 2001 From: nick evans Date: Sun, 19 Jan 2025 12:03:19 -0500 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=94=A7=20Add=20config=20option=20for?= =?UTF-8?q?=20max=20UIDPlusData=20size?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A parser error will be raised when a `uid-set` contains more numbers than `config.parser_max_deprecated_uidplus_data_size`. --- lib/net/imap/config.rb | 34 ++++++++++++++++++++++ lib/net/imap/response_parser.rb | 4 +-- test/net/imap/test_imap_response_parser.rb | 23 +++++++++++++++ 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/lib/net/imap/config.rb b/lib/net/imap/config.rb index d5b0975a..7412fd84 100644 --- a/lib/net/imap/config.rb +++ b/lib/net/imap/config.rb @@ -291,6 +291,12 @@ def self.[](config) # CopyUIDData for +COPYUID+ response codes, and UIDPlusData or # AppendUIDData for +APPENDUID+ response codes. # + # UIDPlusData stores its data in arrays of numbers, which is vulnerable to + # a memory exhaustion denial of service attack from an untrusted or + # compromised server. Set this option to +false+ to completely block this + # vulnerability. Otherwise, parser_max_deprecated_uidplus_data_size + # mitigates this vulnerability. + # # AppendUIDData and CopyUIDData are _mostly_ backward-compatible with # UIDPlusData. Most applications should be able to upgrade with little # or no changes. @@ -313,6 +319,30 @@ def self.[](config) true, false ] + # The maximum +uid-set+ size that ResponseParser will parse into + # deprecated UIDPlusData. This limit only applies when + # parser_use_deprecated_uidplus_data is not +false+. + # + # (Parser support for +UIDPLUS+ added in +v0.3.2+.) + # + # Support for limiting UIDPlusData to a maximum size was added in + # +v0.3.8+, +v0.4.19+, and +v0.5.6+. + # + # UIDPlusData will be removed in +v0.6+. + # + # ==== Versioned Defaults + # + # Because this limit guards against a remote server causing catastrophic + # memory exhaustion, the versioned default (used by #load_defaults) also + # applies to versions without the feature. + # + # * +0.3+ and prior: 10,000 + # * +0.4+: 1,000 + # * +0.5+: 100 + # * +0.6+: 0 + # + attr_accessor :parser_max_deprecated_uidplus_data_size, type: Integer + # Creates a new config object and initialize its attribute with +attrs+. # # If +parent+ is not given, the global config is used by default. @@ -394,6 +424,7 @@ def defaults_hash enforce_logindisabled: true, responses_without_block: :warn, parser_use_deprecated_uidplus_data: true, + parser_max_deprecated_uidplus_data_size: 100, ).freeze @global = default.new @@ -406,6 +437,7 @@ def defaults_hash responses_without_block: :silence_deprecation_warning, enforce_logindisabled: false, parser_use_deprecated_uidplus_data: true, + parser_max_deprecated_uidplus_data_size: 10_000, ).freeze version_defaults[0.0] = Config[0] version_defaults[0.1] = Config[0] @@ -414,6 +446,7 @@ def defaults_hash version_defaults[0.4] = Config[0.3].dup.update( sasl_ir: true, + parser_max_deprecated_uidplus_data_size: 1000, ).freeze version_defaults[0.5] = Config[:current] @@ -421,6 +454,7 @@ def defaults_hash version_defaults[0.6] = Config[0.5].dup.update( responses_without_block: :frozen_dup, parser_use_deprecated_uidplus_data: false, + parser_max_deprecated_uidplus_data_size: 0, ).freeze version_defaults[:next] = Config[0.6] version_defaults[:future] = Config[:next] diff --git a/lib/net/imap/response_parser.rb b/lib/net/imap/response_parser.rb index 826ca527..6fb0d958 100644 --- a/lib/net/imap/response_parser.rb +++ b/lib/net/imap/response_parser.rb @@ -8,8 +8,6 @@ class IMAP < Protocol # Parses an \IMAP server response. class ResponseParser - MAX_UID_SET_SIZE = 10_000 - include ParserUtils extend ParserUtils::Generator @@ -2027,7 +2025,7 @@ def DeprecatedUIDPlus(validity, src_uids = nil, dst_uids) return unless config.parser_use_deprecated_uidplus_data compact_uid_sets = [src_uids, dst_uids].compact count = compact_uid_sets.map { _1.count_with_duplicates }.max - max = MAX_UID_SET_SIZE + max = config.parser_max_deprecated_uidplus_data_size if count <= max src_uids &&= src_uids.each_ordered_number.to_a dst_uids = dst_uids.each_ordered_number.to_a diff --git a/test/net/imap/test_imap_response_parser.rb b/test/net/imap/test_imap_response_parser.rb index 33427e6a..b1e21215 100644 --- a/test/net/imap/test_imap_response_parser.rb +++ b/test/net/imap/test_imap_response_parser.rb @@ -214,12 +214,22 @@ def test_fetch_binary_and_binary_size test "APPENDUID with parser_use_deprecated_uidplus_data = true" do parser = Net::IMAP::ResponseParser.new(config: { parser_use_deprecated_uidplus_data: true, + parser_max_deprecated_uidplus_data_size: 10_000, }) assert_raise_with_message Net::IMAP::ResponseParseError, /uid-set is too large/ do parser.parse( "A004 OK [APPENDUID 1 10000:20000,1] Done\r\n" ) end + response = parser.parse("A004 OK [APPENDUID 1 100:200] Done\r\n") + uidplus = response.data.code.data + assert_equal 101, uidplus.assigned_uids.size + parser.config.parser_max_deprecated_uidplus_data_size = 100 + assert_raise_with_message Net::IMAP::ResponseParseError, /uid-set is too large/ do + parser.parse( + "A004 OK [APPENDUID 1 100:200] Done\r\n" + ) + end response = parser.parse("A004 OK [APPENDUID 1 101:200] Done\r\n") uidplus = response.data.code.data assert_instance_of Net::IMAP::UIDPlusData, uidplus @@ -229,6 +239,7 @@ def test_fetch_binary_and_binary_size test "APPENDUID with parser_use_deprecated_uidplus_data = false" do parser = Net::IMAP::ResponseParser.new(config: { parser_use_deprecated_uidplus_data: false, + parser_max_deprecated_uidplus_data_size: 10_000_000, }) response = parser.parse("A004 OK [APPENDUID 1 10] Done\r\n") assert_instance_of Net::IMAP::AppendUIDData, response.data.code.data @@ -267,12 +278,23 @@ def test_fetch_binary_and_binary_size test "COPYUID with parser_use_deprecated_uidplus_data = true" do parser = Net::IMAP::ResponseParser.new(config: { parser_use_deprecated_uidplus_data: true, + parser_max_deprecated_uidplus_data_size: 10_000, }) assert_raise_with_message Net::IMAP::ResponseParseError, /uid-set is too large/ do parser.parse( "A004 OK [copyUID 1 10000:20000,1 1:10001] Done\r\n" ) end + response = parser.parse("A004 OK [copyUID 1 100:200 1:101] Done\r\n") + uidplus = response.data.code.data + assert_equal 101, uidplus.assigned_uids.size + assert_equal 101, uidplus.source_uids.size + parser.config.parser_max_deprecated_uidplus_data_size = 100 + assert_raise_with_message Net::IMAP::ResponseParseError, /uid-set is too large/ do + parser.parse( + "A004 OK [copyUID 1 100:200 1:101] Done\r\n" + ) + end response = parser.parse("A004 OK [copyUID 1 101:200 1:100] Done\r\n") uidplus = response.data.code.data assert_instance_of Net::IMAP::UIDPlusData, uidplus @@ -283,6 +305,7 @@ def test_fetch_binary_and_binary_size test "COPYUID with parser_use_deprecated_uidplus_data = false" do parser = Net::IMAP::ResponseParser.new(config: { parser_use_deprecated_uidplus_data: false, + parser_max_deprecated_uidplus_data_size: 10_000_000, }) response = parser.parse("A004 OK [COPYUID 1 101 1] Done\r\n") assert_instance_of Net::IMAP::CopyUIDData, response.data.code.data From e58aff64d55dda4215fa0cfd7f4d1ea7b9ca51ba Mon Sep 17 00:00:00 2001 From: nick evans Date: Wed, 22 Jan 2025 17:54:41 -0500 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=94=A7=20Add=20`:up=5Fto=5Fmax=5Fsize?= =?UTF-8?q?`=20config=20for=20UIDPlusData?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When `parser_use_deprecated_uidplus_data` is set to `:up_to_max_size`, ResponseParser uses UIDPlusData when the `uid-set` size is below `parser_max_deprecated_uidplus_data_size`. Above that size, ResponseParser uses AppendUIDData or CopyUIDData. This option is now the default for v0.5. --- lib/net/imap/config.rb | 9 ++++++-- lib/net/imap/response_parser.rb | 2 +- test/net/imap/test_imap_response_parser.rb | 24 ++++++++++++++++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/lib/net/imap/config.rb b/lib/net/imap/config.rb index 7412fd84..1e0300c5 100644 --- a/lib/net/imap/config.rb +++ b/lib/net/imap/config.rb @@ -313,10 +313,15 @@ def self.[](config) # [+true+ (original default)] # ResponseParser only uses UIDPlusData. # + # [+:up_to_max_size+ (default since +v0.5.6+)] + # ResponseParser uses UIDPlusData when the +uid-set+ size is below + # parser_max_deprecated_uidplus_data_size. Above that size, + # ResponseParser uses AppendUIDData or CopyUIDData. + # # [+false+ (planned default for +v0.6+)] # ResponseParser _only_ uses AppendUIDData and CopyUIDData. attr_accessor :parser_use_deprecated_uidplus_data, type: [ - true, false + true, :up_to_max_size, false ] # The maximum +uid-set+ size that ResponseParser will parse into @@ -423,7 +428,7 @@ def defaults_hash sasl_ir: true, enforce_logindisabled: true, responses_without_block: :warn, - parser_use_deprecated_uidplus_data: true, + parser_use_deprecated_uidplus_data: :up_to_max_size, parser_max_deprecated_uidplus_data_size: 100, ).freeze diff --git a/lib/net/imap/response_parser.rb b/lib/net/imap/response_parser.rb index 6fb0d958..03e54b20 100644 --- a/lib/net/imap/response_parser.rb +++ b/lib/net/imap/response_parser.rb @@ -2030,7 +2030,7 @@ def DeprecatedUIDPlus(validity, src_uids = nil, dst_uids) src_uids &&= src_uids.each_ordered_number.to_a dst_uids = dst_uids.each_ordered_number.to_a UIDPlusData.new(validity, src_uids, dst_uids) - else + elsif config.parser_use_deprecated_uidplus_data != :up_to_max_size parse_error("uid-set is too large: %d > %d", count, max) end end diff --git a/test/net/imap/test_imap_response_parser.rb b/test/net/imap/test_imap_response_parser.rb index b1e21215..fa6ba757 100644 --- a/test/net/imap/test_imap_response_parser.rb +++ b/test/net/imap/test_imap_response_parser.rb @@ -236,6 +236,17 @@ def test_fetch_binary_and_binary_size assert_equal 100, uidplus.assigned_uids.size end + test "APPENDUID with parser_use_deprecated_uidplus_data = :up_to_max_size" do + parser = Net::IMAP::ResponseParser.new(config: { + parser_use_deprecated_uidplus_data: :up_to_max_size, + parser_max_deprecated_uidplus_data_size: 100 + }) + response = parser.parse("A004 OK [APPENDUID 1 101:200] Done\r\n") + assert_instance_of Net::IMAP::UIDPlusData, response.data.code.data + response = parser.parse("A004 OK [APPENDUID 1 100:200] Done\r\n") + assert_instance_of Net::IMAP::AppendUIDData, response.data.code.data + end + test "APPENDUID with parser_use_deprecated_uidplus_data = false" do parser = Net::IMAP::ResponseParser.new(config: { parser_use_deprecated_uidplus_data: false, @@ -302,6 +313,19 @@ def test_fetch_binary_and_binary_size assert_equal 100, uidplus.source_uids.size end + test "COPYUID with parser_use_deprecated_uidplus_data = :up_to_max_size" do + parser = Net::IMAP::ResponseParser.new(config: { + parser_use_deprecated_uidplus_data: :up_to_max_size, + parser_max_deprecated_uidplus_data_size: 100 + }) + response = parser.parse("A004 OK [COPYUID 1 101:200 1:100] Done\r\n") + copyuid = response.data.code.data + assert_instance_of Net::IMAP::UIDPlusData, copyuid + response = parser.parse("A004 OK [COPYUID 1 100:200 1:101] Done\r\n") + copyuid = response.data.code.data + assert_instance_of Net::IMAP::CopyUIDData, copyuid + end + test "COPYUID with parser_use_deprecated_uidplus_data = false" do parser = Net::IMAP::ResponseParser.new(config: { parser_use_deprecated_uidplus_data: false,