Skip to content

Commit b3ad198

Browse files
committed
🍒 pick 24d5c77: 🔒🥅 Handle tagged "OK" to incomplete command [backport #664]
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 7a233c5 commit b3ad198

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
@@ -3141,6 +3141,7 @@ def idle(timeout = nil, &response_handler)
31413141

31423142
synchronize do
31433143
tag = Thread.current[:net_imap_tag] = generate_tag
3144+
guard_against_tagged_response_skipping_handler!(tag, "IDLE")
31443145
put_string("#{tag} IDLE#{CRLF}")
31453146

31463147
begin
@@ -3605,6 +3606,7 @@ def send_command(cmd, *args, &block)
36053606
put_string(" ")
36063607
send_data(i, tag)
36073608
end
3609+
guard_against_tagged_response_skipping_handler!(tag, cmd)
36083610
put_string(CRLF)
36093611
if cmd == "LOGOUT"
36103612
@logout_command_tag = tag
@@ -3620,6 +3622,19 @@ def send_command(cmd, *args, &block)
36203622
end
36213623
end
36223624
end
3625+
rescue InvalidResponseError
3626+
disconnect
3627+
raise
3628+
end
3629+
3630+
def guard_against_tagged_response_skipping_handler!(tag, cmd)
3631+
return unless (resp = @tagged_responses[tag])&.name&.upcase == "OK"
3632+
raise InvalidResponseError, format(
3633+
"Received tagged 'OK' to incomplete %s command (tag=%s). " \
3634+
"This could indicate a malicious server, a man-in-the-middle, or " \
3635+
"client-side command injection. Disconnecting.",
3636+
cmd, tag
3637+
)
36233638
end
36243639

36253640
def generate_tag

0 commit comments

Comments
 (0)