Skip to content

Commit 24d5c77

Browse files
committed
🔒🥅 Handle tagged "OK" to incomplete command
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.
1 parent 62eea6f commit 24d5c77

1 file changed

Lines changed: 15 additions & 0 deletions

File tree

lib/net/imap.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3174,6 +3174,7 @@ def idle(timeout = nil, &response_handler)
31743174

31753175
synchronize do
31763176
tag = Thread.current[:net_imap_tag] = generate_tag
3177+
guard_against_tagged_response_skipping_handler!(tag, "IDLE")
31773178
put_string("#{tag} IDLE#{CRLF}")
31783179

31793180
begin
@@ -3638,6 +3639,7 @@ def send_command(cmd, *args, &block)
36383639
put_string(" ")
36393640
send_data(i, tag)
36403641
end
3642+
guard_against_tagged_response_skipping_handler!(tag, cmd)
36413643
put_string(CRLF)
36423644
if cmd == "LOGOUT"
36433645
@logout_command_tag = tag
@@ -3653,6 +3655,19 @@ def send_command(cmd, *args, &block)
36533655
end
36543656
end
36553657
end
3658+
rescue InvalidResponseError
3659+
disconnect
3660+
raise
3661+
end
3662+
3663+
def guard_against_tagged_response_skipping_handler!(tag, cmd)
3664+
return unless (resp = @tagged_responses[tag])&.name&.upcase == "OK"
3665+
raise InvalidResponseError, format(
3666+
"Received tagged 'OK' to incomplete %s command (tag=%s). " \
3667+
"This could indicate a malicious server, a man-in-the-middle, or " \
3668+
"client-side command injection. Disconnecting.",
3669+
cmd, tag
3670+
)
36563671
end
36573672

36583673
def generate_tag

0 commit comments

Comments
 (0)