|
|
20333d |
From fb263d33f99b5d8c8f370df48a04c926eac5781e Mon Sep 17 00:00:00 2001
|
|
|
4c08dd |
From: Christophe Fergeau <cfergeau@redhat.com>
|
|
|
4c08dd |
Date: Fri, 23 Aug 2013 11:29:44 +0200
|
|
|
4c08dd |
Subject: [PATCH] Fix buffer overflow when decrypting client SPICE ticket
|
|
|
4c08dd |
|
|
|
4c08dd |
reds_handle_ticket uses a fixed size 'password' buffer for the decrypted
|
|
|
4c08dd |
password whose size is SPICE_MAX_PASSWORD_LENGTH. However,
|
|
|
4c08dd |
RSA_private_decrypt which we call for the decryption expects the
|
|
|
4c08dd |
destination buffer to be at least RSA_size(link->tiTicketing.rsa)
|
|
|
4c08dd |
bytes long. On my spice-server build, SPICE_MAX_PASSWORD_LENGTH
|
|
|
4c08dd |
is 60 while RSA_size() is 128, so we end up overflowing 'password'
|
|
|
4c08dd |
when using long passwords (this was reproduced using the string:
|
|
|
4c08dd |
'fullscreen=1proxy=#enter proxy here; e.g spice_proxy = http://[proxy]:[port]'
|
|
|
4c08dd |
as a password).
|
|
|
4c08dd |
|
|
|
4c08dd |
When the overflow occurs, QEMU dies with:
|
|
|
4c08dd |
*** stack smashing detected ***: qemu-system-x86_64 terminated
|
|
|
4c08dd |
|
|
|
4c08dd |
This commit ensures we use a corectly sized 'password' buffer,
|
|
|
4c08dd |
and that it's correctly nul-terminated so that we can use strcmp
|
|
|
4c08dd |
instead of strncmp. To keep using strncmp, we'd need to figure out
|
|
|
4c08dd |
which one of 'password' and 'taTicket.password' is the smaller buffer,
|
|
|
4c08dd |
and use that size.
|
|
|
4c08dd |
|
|
|
4c08dd |
This fixes rhbz#999839
|
|
|
4c08dd |
---
|
|
|
4c08dd |
server/reds.c | 44 ++++++++++++++++++++++++++++++++------------
|
|
|
4c08dd |
1 file changed, 32 insertions(+), 12 deletions(-)
|
|
|
4c08dd |
|
|
|
4c08dd |
diff --git a/server/reds.c b/server/reds.c
|
|
|
20333d |
index ae87c90..f9f185d 100644
|
|
|
4c08dd |
--- a/server/reds.c
|
|
|
4c08dd |
+++ b/server/reds.c
|
|
|
20333d |
@@ -1933,39 +1933,59 @@ static void reds_handle_link(RedLinkInfo *link)
|
|
|
4c08dd |
static void reds_handle_ticket(void *opaque)
|
|
|
4c08dd |
{
|
|
|
4c08dd |
RedLinkInfo *link = (RedLinkInfo *)opaque;
|
|
|
4c08dd |
- char password[SPICE_MAX_PASSWORD_LENGTH];
|
|
|
4c08dd |
+ char *password;
|
|
|
4c08dd |
time_t ltime;
|
|
|
4c08dd |
+ int password_size;
|
|
|
4c08dd |
|
|
|
4c08dd |
//todo: use monotonic time
|
|
|
4c08dd |
time(<ime);
|
|
|
4c08dd |
- RSA_private_decrypt(link->tiTicketing.rsa_size,
|
|
|
4c08dd |
- link->tiTicketing.encrypted_ticket.encrypted_data,
|
|
|
4c08dd |
- (unsigned char *)password, link->tiTicketing.rsa, RSA_PKCS1_OAEP_PADDING);
|
|
|
4c08dd |
+ if (RSA_size(link->tiTicketing.rsa) < SPICE_MAX_PASSWORD_LENGTH) {
|
|
|
4c08dd |
+ spice_warning("RSA modulus size is smaller than SPICE_MAX_PASSWORD_LENGTH (%d < %d), "
|
|
|
4c08dd |
+ "SPICE ticket sent from client may be truncated",
|
|
|
4c08dd |
+ RSA_size(link->tiTicketing.rsa), SPICE_MAX_PASSWORD_LENGTH);
|
|
|
4c08dd |
+ }
|
|
|
4c08dd |
+
|
|
|
4c08dd |
+ password = g_malloc0(RSA_size(link->tiTicketing.rsa) + 1);
|
|
|
4c08dd |
+ password_size = RSA_private_decrypt(link->tiTicketing.rsa_size,
|
|
|
4c08dd |
+ link->tiTicketing.encrypted_ticket.encrypted_data,
|
|
|
4c08dd |
+ (unsigned char *)password,
|
|
|
4c08dd |
+ link->tiTicketing.rsa,
|
|
|
4c08dd |
+ RSA_PKCS1_OAEP_PADDING);
|
|
|
4c08dd |
+ if (password_size == -1) {
|
|
|
4c08dd |
+ spice_warning("failed to decrypt RSA encrypted password: %s",
|
|
|
4c08dd |
+ ERR_error_string(ERR_get_error(), NULL));
|
|
|
4c08dd |
+ goto error;
|
|
|
4c08dd |
+ }
|
|
|
4c08dd |
+ password[password_size] = '\0';
|
|
|
4c08dd |
|
|
|
4c08dd |
if (ticketing_enabled && !link->skip_auth) {
|
|
|
4c08dd |
int expired = taTicket.expiration_time < ltime;
|
|
|
4c08dd |
|
|
|
4c08dd |
if (strlen(taTicket.password) == 0) {
|
|
|
4c08dd |
- reds_send_link_result(link, SPICE_LINK_ERR_PERMISSION_DENIED);
|
|
|
4c08dd |
spice_warning("Ticketing is enabled, but no password is set. "
|
|
|
4c08dd |
- "please set a ticket first");
|
|
|
4c08dd |
- reds_link_free(link);
|
|
|
4c08dd |
- return;
|
|
|
4c08dd |
+ "please set a ticket first");
|
|
|
4c08dd |
+ goto error;
|
|
|
4c08dd |
}
|
|
|
4c08dd |
|
|
|
4c08dd |
- if (expired || strncmp(password, taTicket.password, SPICE_MAX_PASSWORD_LENGTH) != 0) {
|
|
|
4c08dd |
+ if (expired || strcmp(password, taTicket.password) != 0) {
|
|
|
4c08dd |
if (expired) {
|
|
|
4c08dd |
spice_warning("Ticket has expired");
|
|
|
4c08dd |
} else {
|
|
|
4c08dd |
spice_warning("Invalid password");
|
|
|
4c08dd |
}
|
|
|
4c08dd |
- reds_send_link_result(link, SPICE_LINK_ERR_PERMISSION_DENIED);
|
|
|
4c08dd |
- reds_link_free(link);
|
|
|
4c08dd |
- return;
|
|
|
4c08dd |
+ goto error;
|
|
|
4c08dd |
}
|
|
|
4c08dd |
}
|
|
|
4c08dd |
|
|
|
4c08dd |
reds_handle_link(link);
|
|
|
4c08dd |
+ goto end;
|
|
|
4c08dd |
+
|
|
|
4c08dd |
+error:
|
|
|
4c08dd |
+ reds_send_link_result(link, SPICE_LINK_ERR_PERMISSION_DENIED);
|
|
|
4c08dd |
+ reds_link_free(link);
|
|
|
4c08dd |
+
|
|
|
4c08dd |
+end:
|
|
|
4c08dd |
+ g_free(password);
|
|
|
4c08dd |
}
|
|
|
4c08dd |
|
|
|
4c08dd |
static inline void async_read_clear_handlers(AsyncRead *obj)
|