Skip to content

Commit 038ae35

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 the sending the command has completed.
1 parent 705aa59 commit 038ae35

1 file changed

Lines changed: 12 additions & 0 deletions

File tree

lib/net/imap.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3061,6 +3061,7 @@ def send_command(cmd, *args, &block)
30613061
put_string(" ")
30623062
send_data(i, tag)
30633063
end
3064+
guard_against_tagged_response_skipping_handler!(tag)
30643065
put_string(CRLF)
30653066
if cmd == "LOGOUT"
30663067
@logout_command_tag = tag
@@ -3076,6 +3077,17 @@ def send_command(cmd, *args, &block)
30763077
end
30773078
end
30783079
end
3080+
rescue InvalidResponseError
3081+
disconnect
3082+
raise
3083+
end
3084+
3085+
def guard_against_tagged_response_skipping_handler!(tag)
3086+
return unless (resp = @tagged_responses[tag])&.name&.upcase == "OK"
3087+
raise(InvalidResponseError,
3088+
"Server sent tagged 'OK' before command was finished: %p. " \
3089+
"This could indicate a malicious server or client-side " \
3090+
"command injection. Disconnecting." % [resp.raw_data.chomp])
30793091
end
30803092

30813093
def generate_tag

0 commit comments

Comments
 (0)