126 lines
4.1 KiB
Diff
126 lines
4.1 KiB
Diff
From 62eea6ffe1e390060065169474f97edbc42bd2b2 Mon Sep 17 00:00:00 2001
|
|
From: nick evans <nick@rubinick.dev>
|
|
Date: Fri, 27 Mar 2026 17:31:11 -0400
|
|
Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=94=92=F0=9F=A5=85=20Ensure=20STARTTL?=
|
|
=?UTF-8?q?S=20tagged=20response=20was=20handled?=
|
|
MIME-Version: 1.0
|
|
Content-Type: text/plain; charset=UTF-8
|
|
Content-Transfer-Encoding: 8bit
|
|
|
|
Taking a "belt-and-suspenders" approach to a STARTTLS stripping attack:
|
|
|
|
This handles `STARTTLS` as a special-case: if the `STARTTLS` handler
|
|
did not run, for _whatever_ reason, an exception _must_ be raised and
|
|
the connection dropped.
|
|
|
|
_No_ command should ever receive a tagged `OK` prior to completely
|
|
sending the command. But `STARTTLS` is security-sensitive enough to
|
|
warrant this special-case handler.
|
|
---
|
|
lib/net/imap.rb | 9 +++++++++
|
|
1 file changed, 9 insertions(+)
|
|
|
|
diff --git a/lib/net/imap.rb b/lib/net/imap.rb
|
|
index 00f1588..e2cbc5a 100644
|
|
--- a/lib/net/imap.rb
|
|
+++ b/lib/net/imap.rb
|
|
@@ -1390,9 +1390,11 @@ module Net
|
|
#
|
|
def starttls(**options)
|
|
@ssl_ctx_params, @ssl_ctx = build_ssl_ctx(options)
|
|
+ handled = false
|
|
error = nil
|
|
ok = send_command("STARTTLS") do |resp|
|
|
if resp.kind_of?(TaggedResponse) && resp.name == "OK"
|
|
+ handled = true
|
|
clear_cached_capabilities
|
|
clear_responses
|
|
start_tls_session
|
|
@@ -1404,6 +1406,13 @@ module Net
|
|
disconnect
|
|
raise error
|
|
end
|
|
+ unless handled
|
|
+ disconnect
|
|
+ raise InvalidResponseError,
|
|
+ "STARTTLS handler was bypassed, although server responded %p" % [
|
|
+ ok.raw_data.chomp
|
|
+ ]
|
|
+ end
|
|
ok
|
|
end
|
|
|
|
|
|
|
|
From 24d5c773d1bb76ca1cd0a26b2218195011c16969 Mon Sep 17 00:00:00 2001
|
|
From: nick evans <nick@rubinick.dev>
|
|
Date: Fri, 27 Mar 2026 18:00:09 -0400
|
|
Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=94=92=F0=9F=A5=85=20Handle=20tagged?=
|
|
=?UTF-8?q?=20"OK"=20to=20incomplete=20command?=
|
|
MIME-Version: 1.0
|
|
Content-Type: text/plain; charset=UTF-8
|
|
Content-Transfer-Encoding: 8bit
|
|
|
|
Taking a "belt-and-suspenders" approach:
|
|
|
|
This is a potential problem for any command which registers a response
|
|
handler: a malicious server can easily guess what the next tag will be,
|
|
and send an `OK` response _before_ the client the response handler is
|
|
attached.
|
|
|
|
`STARTTLS` is an extreme example of this issue: if the `STARTTLS`
|
|
handler does not run, then `#starttls` will not start the TLS session,
|
|
and the connection is not secured, _but no error is raised._
|
|
|
|
We should _also_ attach the response handler before sending the `CRLF`,
|
|
but that is neither necessary (the response handler will added before
|
|
the `synchronize` mutex is unlocked) nor sufficient (the fake `OK` can
|
|
be sent _much_ earlier).
|
|
|
|
On the other hand, it _is_ okay for the server to send an error tagged
|
|
response (`NO` or `BAD`), before sending the command has completed.
|
|
---
|
|
lib/net/imap.rb | 15 +++++++++++++++
|
|
1 file changed, 15 insertions(+)
|
|
|
|
diff --git a/lib/net/imap.rb b/lib/net/imap.rb
|
|
index e2cbc5a..6af1ca5 100644
|
|
--- a/lib/net/imap.rb
|
|
+++ b/lib/net/imap.rb
|
|
@@ -3086,6 +3086,7 @@ module Net
|
|
|
|
synchronize do
|
|
tag = Thread.current[:net_imap_tag] = generate_tag
|
|
+ guard_against_tagged_response_skipping_handler!(tag, "IDLE")
|
|
put_string("#{tag} IDLE#{CRLF}")
|
|
|
|
begin
|
|
@@ -3550,6 +3551,7 @@ module Net
|
|
put_string(" ")
|
|
send_data(i, tag)
|
|
end
|
|
+ guard_against_tagged_response_skipping_handler!(tag, cmd)
|
|
put_string(CRLF)
|
|
if cmd == "LOGOUT"
|
|
@logout_command_tag = tag
|
|
@@ -3565,6 +3567,19 @@ module Net
|
|
end
|
|
end
|
|
end
|
|
+ rescue InvalidResponseError
|
|
+ disconnect
|
|
+ raise
|
|
+ end
|
|
+
|
|
+ def guard_against_tagged_response_skipping_handler!(tag, cmd)
|
|
+ return unless (resp = @tagged_responses[tag])&.name&.upcase == "OK"
|
|
+ raise InvalidResponseError, format(
|
|
+ "Received tagged 'OK' to incomplete %s command (tag=%s). " \
|
|
+ "This could indicate a malicious server, a man-in-the-middle, or " \
|
|
+ "client-side command injection. Disconnecting.",
|
|
+ cmd, tag
|
|
+ )
|
|
end
|
|
|
|
def generate_tag
|