From bcb124ce4fe8f30a27273585c56a128376c09b81 Mon Sep 17 00:00:00 2001 From: Christophe Fergeau Date: Wed, 30 Oct 2013 11:36:24 +0100 Subject: [PATCH] Add patch fixing CVE-2013-4282 --- ...low-when-decrypting-client-SPICE-tic.patch | 105 ++++++++++++++++++ spice.spec | 7 +- 2 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 0010-Fix-buffer-overflow-when-decrypting-client-SPICE-tic.patch diff --git a/0010-Fix-buffer-overflow-when-decrypting-client-SPICE-tic.patch b/0010-Fix-buffer-overflow-when-decrypting-client-SPICE-tic.patch new file mode 100644 index 0000000..8e957af --- /dev/null +++ b/0010-Fix-buffer-overflow-when-decrypting-client-SPICE-tic.patch @@ -0,0 +1,105 @@ +From 8af619009660b24e0b41ad26b30289eea288fcc2 Mon Sep 17 00:00:00 2001 +From: Christophe Fergeau +Date: Fri, 23 Aug 2013 11:29:44 +0200 +Subject: [PATCH] Fix buffer overflow when decrypting client SPICE ticket + +reds_handle_ticket uses a fixed size 'password' buffer for the decrypted +password whose size is SPICE_MAX_PASSWORD_LENGTH. However, +RSA_private_decrypt which we call for the decryption expects the +destination buffer to be at least RSA_size(link->tiTicketing.rsa) +bytes long. On my spice-server build, SPICE_MAX_PASSWORD_LENGTH +is 60 while RSA_size() is 128, so we end up overflowing 'password' +when using long passwords (this was reproduced using the string: +'fullscreen=1proxy=#enter proxy here; e.g spice_proxy = http://[proxy]:[port]' +as a password). + +When the overflow occurs, QEMU dies with: +*** stack smashing detected ***: qemu-system-x86_64 terminated + +This commit ensures we use a corectly sized 'password' buffer, +and that it's correctly nul-terminated so that we can use strcmp +instead of strncmp. To keep using strncmp, we'd need to figure out +which one of 'password' and 'taTicket.password' is the smaller buffer, +and use that size. + +This fixes rhbz#999839 +--- + server/reds.c | 44 ++++++++++++++++++++++++++++++++------------ + 1 file changed, 32 insertions(+), 12 deletions(-) + +diff --git a/server/reds.c b/server/reds.c +index 892d247..2a0002b 100644 +--- a/server/reds.c ++++ b/server/reds.c +@@ -1926,39 +1926,59 @@ static void reds_handle_link(RedLinkInfo *link) + static void reds_handle_ticket(void *opaque) + { + RedLinkInfo *link = (RedLinkInfo *)opaque; +- char password[SPICE_MAX_PASSWORD_LENGTH]; ++ char *password; + time_t ltime; ++ int password_size; + + //todo: use monotonic time + time(<ime); +- RSA_private_decrypt(link->tiTicketing.rsa_size, +- link->tiTicketing.encrypted_ticket.encrypted_data, +- (unsigned char *)password, link->tiTicketing.rsa, RSA_PKCS1_OAEP_PADDING); ++ if (RSA_size(link->tiTicketing.rsa) < SPICE_MAX_PASSWORD_LENGTH) { ++ spice_warning("RSA modulus size is smaller than SPICE_MAX_PASSWORD_LENGTH (%d < %d), " ++ "SPICE ticket sent from client may be truncated", ++ RSA_size(link->tiTicketing.rsa), SPICE_MAX_PASSWORD_LENGTH); ++ } ++ ++ password = g_malloc0(RSA_size(link->tiTicketing.rsa) + 1); ++ password_size = RSA_private_decrypt(link->tiTicketing.rsa_size, ++ link->tiTicketing.encrypted_ticket.encrypted_data, ++ (unsigned char *)password, ++ link->tiTicketing.rsa, ++ RSA_PKCS1_OAEP_PADDING); ++ if (password_size == -1) { ++ spice_warning("failed to decrypt RSA encrypted password: %s", ++ ERR_error_string(ERR_get_error(), NULL)); ++ goto error; ++ } ++ password[password_size] = '\0'; + + if (ticketing_enabled && !link->skip_auth) { + int expired = taTicket.expiration_time < ltime; + + if (strlen(taTicket.password) == 0) { +- reds_send_link_result(link, SPICE_LINK_ERR_PERMISSION_DENIED); + spice_warning("Ticketing is enabled, but no password is set. " +- "please set a ticket first"); +- reds_link_free(link); +- return; ++ "please set a ticket first"); ++ goto error; + } + +- if (expired || strncmp(password, taTicket.password, SPICE_MAX_PASSWORD_LENGTH) != 0) { ++ if (expired || strcmp(password, taTicket.password) != 0) { + if (expired) { + spice_warning("Ticket has expired"); + } else { + spice_warning("Invalid password"); + } +- reds_send_link_result(link, SPICE_LINK_ERR_PERMISSION_DENIED); +- reds_link_free(link); +- return; ++ goto error; + } + } + + reds_handle_link(link); ++ goto end; ++ ++error: ++ reds_send_link_result(link, SPICE_LINK_ERR_PERMISSION_DENIED); ++ reds_link_free(link); ++ ++end: ++ g_free(password); + } + + static inline void async_read_clear_handlers(AsyncRead *obj) diff --git a/spice.spec b/spice.spec index 3b6a2c8..cec90ee 100644 --- a/spice.spec +++ b/spice.spec @@ -1,6 +1,6 @@ Name: spice Version: 0.12.4 -Release: 2%{?dist} +Release: 3%{?dist} Summary: Implements the SPICE protocol Group: User Interface/Desktops License: LGPLv2+ @@ -15,6 +15,7 @@ Patch6: 0006-snd_worker-fix-memory-leak-of-PlaybackChannel.patch Patch7: 0007-snd_worker-snd_disconnect_channel-don-t-call-snd_cha.patch Patch8: 0008-log-improve-debug-information-related-to-client-disc.patch Patch9: 0009-red_worker-decrease-the-timeout-when-flushing-comman.patch +Patch10: 0010-Fix-buffer-overflow-when-decrypting-client-SPICE-tic.patch # https://bugzilla.redhat.com/show_bug.cgi?id=613529 %if 0%{?rhel} @@ -78,6 +79,7 @@ using spice-server, you will need to install spice-server-devel. %patch7 -p1 %patch8 -p1 %patch9 -p1 +%patch10 -p1 %build @@ -108,6 +110,9 @@ mkdir -p %{buildroot}%{_libexecdir} %changelog +* Wed Oct 30 2013 Christophe Fergeau 0.12.4-3 +- Add patch fixing CVE-2013-4282 + * Fri Sep 13 2013 Christophe Fergeau 0.12.4-2 - Add upstream patch fixing rhbz#995041