124 lines
4.9 KiB
Diff
124 lines
4.9 KiB
Diff
From 0dbb8eb72aa78b7ed0b01533388e69722d16b821 Mon Sep 17 00:00:00 2001
|
|
From: nick evans <nick@rubinick.dev>
|
|
Date: Wed, 8 Jan 2025 22:06:47 -0500
|
|
Subject: [PATCH 3/5] =?UTF-8?q?=F0=9F=94=92=20Prevent=20runaway=20memory?=
|
|
=?UTF-8?q?=20use=20when=20parsing=20uid-set?=
|
|
MIME-Version: 1.0
|
|
Content-Type: text/plain; charset=UTF-8
|
|
Content-Transfer-Encoding: 8bit
|
|
|
|
Reference:https://github.com/ruby/net-imap/commit/0dbb8eb72aa78b7ed0b01533388e69722d16b821
|
|
Conflict:not change test, the test/net/imap/ directory doesn't exist.
|
|
|
|
Problem
|
|
=========
|
|
|
|
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"`
|
|
|
|
Preferred fix
|
|
===============
|
|
|
|
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.
|
|
|
|
Workaround
|
|
============
|
|
|
|
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 (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
|
|
maximum array size is configurable, with a much lower default: 1000 for
|
|
v0.4 and 100 for v0.5. These are low enough that they are _unlikely_ to
|
|
cause a problem, but v0.4 and v0.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 | 26 ++++++++++++++++---
|
|
1 file changed, 23 insertions(+), 3 deletions(-)
|
|
|
|
diff --git a/.bundle/gems/net-imap-0.3.4/lib/net/imap/response_parser.rb b/.bundle/gems/net-imap-0.3.4/lib/net/imap/response_parser.rb
|
|
index be1a849..0341356 100644
|
|
--- a/.bundle/gems/net-imap-0.3.4/lib/net/imap/response_parser.rb
|
|
+++ b/.bundle/gems/net-imap-0.3.4/lib/net/imap/response_parser.rb
|
|
@@ -7,6 +7,8 @@ module Net
|
|
|
|
# Parses an \IMAP server response.
|
|
class ResponseParser
|
|
+ MAX_UID_SET_SIZE = 10_000
|
|
+
|
|
# :call-seq: Net::IMAP::ResponseParser.new -> Net::IMAP::ResponseParser
|
|
def initialize
|
|
@str = nil
|
|
@@ -1379,11 +1381,29 @@ module Net
|
|
case token.symbol
|
|
when T_NUMBER then [Integer(token.value)]
|
|
when T_ATOM
|
|
- token.value.split(",").flat_map {|range|
|
|
- range = range.split(":").map {|uniqueid| Integer(uniqueid) }
|
|
- range.size == 1 ? range : Range.new(range.min, range.max).to_a
|
|
+ entries = uid_set__ranges(token.value)
|
|
+ if (count = entries.sum(&:count)) > MAX_UID_SET_SIZE
|
|
+ parse_error("uid-set is too large: %d > 10k", count)
|
|
+ end
|
|
+ entries.flat_map(&:to_a)
|
|
+ end
|
|
+ end
|
|
+
|
|
+ # returns an array of ranges
|
|
+ def uid_set__ranges(uidset)
|
|
+ entries = []
|
|
+ uidset.split(",") do |entry|
|
|
+ uids = entry.split(":", 2).map {|uid|
|
|
+ unless uid =~ /\A[1-9][0-9]*\z/
|
|
+ parse_error("invalid uid-set uid: %p", uid)
|
|
+ end
|
|
+ uid = Integer(uid)
|
|
+ NumValidator.ensure_nz_number(uid)
|
|
+ uid
|
|
}
|
|
+ entries << Range.new(*uids.minmax)
|
|
end
|
|
+ entries
|
|
end
|
|
|
|
def nil_atom
|
|
--
|
|
2.27.0
|
|
|