Repository navigation
Fixed heap buffer overflow decrypting server counter-challenge - #6508
Open
Dhanu-dynamic wants to merge 1 commit into
Open
Dhanu-dynamic wants to merge 1 commit into
Dhanu-dynamic wants to merge 1 commit into
Conversation
In the classic protocol key exchange, AuthenticateAgent() allocated the
buffer for the decrypted counter-challenge with the length of the
ciphertext the server sent:
decrypted_cchall = xmalloc(encrypted_len);
RSA_private_decrypt(encrypted_len, in, decrypted_cchall, PRIVKEY, ...)
RSA_private_decrypt() can write up to RSA_size(PRIVKEY) bytes, and its
documentation requires an output buffer that large; the ciphertext
length is whatever the server chose to send. With OpenSSL 3.2 and later,
PKCS#1 v1.5 decryption uses implicit rejection and returns a synthetic
message of pseudo-random length instead of failing on bad padding, so a
server that answers with a 1 byte counter-challenge makes the agent
write up to about 240 bytes past a 1 byte allocation (2048-bit key).
The digest that follows also read nonce_len bytes from that buffer no
matter how much had been decrypted.
Size the buffer with RSA_size(PRIVKEY), refuse a counter-challenge that
decrypts to fewer bytes than the nonce being hashed, and free the buffer
on the error paths.
Changelog: Title
Signed-off-by: Dhanraj E <dhanraj@labs.digiscrypt.com>
| { | ||
| Log(LOG_LEVEL_ERR, | ||
| "Counter-challenge from server '%s' is too short (%d bytes, expected %d)", | ||
| conn->this_server, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AuthenticateAgent() allocates the buffer for the decrypted counter-challenge (proposition S3 of the classic key exchange) with the length of the ciphertext the server sent, but RSA_private_decrypt() may write up to RSA_size(PRIVKEY) bytes. since openssl 3.2, pkcs#1 v1.5 implicit rejection returns a synthetic message of random length instead of an error, so a server that replies with a 1 byte counter-challenge overflows the heap of cf-agent, cf-runagent or cf-net. with a fake server on a socketpair driving the real AuthenticateAgent() against openssl 3.3.2, up to 187 bytes were written into a 16 byte block and the client died with heap corruption in 5 of 6 runs; with this change 10 of 10 runs stayed inside the buffer and authentication failed cleanly. getting to S3 needs protocol_version classic and a server that either holds the key the client already trusts (for example a compromised host that cf-runagent connects to) or whose unknown key the client is willing to trust. no unit test, since the write happens inside libcrypto where a test can't observe it reliably; happy to share the reproducer.