Skip to content

Commit 62eea6f

Browse files
committed
🔒🥅 Ensure STARTTLS tagged response was handled
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.
1 parent 46636ca commit 62eea6f

1 file changed

Lines changed: 9 additions & 0 deletions

File tree

lib/net/imap.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1440,9 +1440,11 @@ def logout!
14401440
#
14411441
def starttls(**options)
14421442
@ssl_ctx_params, @ssl_ctx = build_ssl_ctx(options)
1443+
handled = false
14431444
error = nil
14441445
ok = send_command("STARTTLS") do |resp|
14451446
if resp.kind_of?(TaggedResponse) && resp.name == "OK"
1447+
handled = true
14461448
clear_cached_capabilities
14471449
clear_responses
14481450
start_tls_session
@@ -1454,6 +1456,13 @@ def starttls(**options)
14541456
disconnect
14551457
raise error
14561458
end
1459+
unless handled
1460+
disconnect
1461+
raise InvalidResponseError,
1462+
"STARTTLS handler was bypassed, although server responded %p" % [
1463+
ok.raw_data.chomp
1464+
]
1465+
end
14571466
ok
14581467
end
14591468

0 commit comments

Comments
 (0)