Skip to content

Commit 567a231

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 b01e0d7 commit 567a231

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
@@ -2311,6 +2311,7 @@ def send_command(cmd, *args, &block)
23112311
put_string(" ")
23122312
send_data(i, tag)
23132313
end
2314+
guard_against_tagged_response_skipping_handler!(tag)
23142315
put_string(CRLF)
23152316
if cmd == "LOGOUT"
23162317
@logout_command_tag = tag
@@ -2326,6 +2327,17 @@ def send_command(cmd, *args, &block)
23262327
end
23272328
end
23282329
end
2330+
rescue InvalidResponseError
2331+
disconnect
2332+
raise
2333+
end
2334+
2335+
def guard_against_tagged_response_skipping_handler!(tag)
2336+
return unless (resp = @tagged_responses[tag])&.name&.upcase == "OK"
2337+
raise(InvalidResponseError,
2338+
"Server sent tagged 'OK' before command was finished: %p. " \
2339+
"This could indicate a malicious server or client-side " \
2340+
"command injection. Disconnecting." % [resp.raw_data.chomp])
23292341
end
23302342

23312343
def generate_tag

0 commit comments

Comments
 (0)