Hello again smile

Context as usual:
latest git, sles 11.1 and macosx 10.7.4

./configure --prefix=/usr/local/cyassl --enable-hugecache --enable-fastmath --enable-debug

start server (-d is important in this case, since message is ad-hoc)

./examples/server/.libs/server -p 1105 -v 1 -d

send the following handshake message

echo -ne '16030101061000010201008ae7eddc582257a9e6f26cd61c529166b46ab9c1b2a652066a1389672afce29ae8d1f7dc7a39d573fb048b5c193bf87cc8b943f0bc13c3d9c09b5ec5dca38dbdded583f13b4a8eec3fa089bcd8e6f27b9f7587ae4b507b9f3f324c03b7c2489762d64fb5288afdb49a300022370fff7bacfb2b41b97aa4a8eda14eff85df4333887ca8d7e5dbe7de509d6f3cfe3f1f1e42355f49797a995d27225523ca75e43157a573f63514b464401591adfba3edf2414a43af7ec35cecf7f020ec3ec697c86373f507f087e2ae11412e89a6cb6356dcc709e3899caac68f597bdfa38b1c5cb1339045e2db528a211fff4ac915dd91fb0ab0a872ee2f0081f970468d64d4e71403010001011603010030f2ea69c2f09920c02ec2a278454c233516ae19ef99c105caae50ba493c026c4e0b27947a3d5a40511db89a7be4713584' | xxd -r -p | netcat localhost 1105 | xxd

log + valgrind complains (and since my app crashes around there - i assume nasty things happen from wrong keys being initialized, etc.)

<skipped startup stuff>

CyaSSL Entering SSL_set_fd
CyaSSL Leaving SSL_set_fd, return 1
CyaSSL Entering CyaSSL_SetTmpDH
CyaSSL Leaving CyaSSL_SetTmpDH, return 0
CyaSSL Entering SSL_accept()
growing input buffer

growing input buffer

received record layer msg
CyaSSL Entering DoHandShakeMsg()
CyaSSL Entering DoHandShakeMsgType
processing client key exchange

==58560== Conditional jump or move depends on uninitialised value(s)
==58560==    at 0x17FC2: DoClientKeyExchange (in /usr/local/cyassl/lib/libcyassl.3.dylib)
==58560==    by 0xE36D: DoHandShakeMsgType (in /usr/local/cyassl/lib/libcyassl.3.dylib)
==58560==    by 0xE50F: DoHandShakeMsg (in /usr/local/cyassl/lib/libcyassl.3.dylib)
==58560==    by 0xF515: ProcessReply (in /usr/local/cyassl/lib/libcyassl.3.dylib)
==58560==    by 0x1EE6C: CyaSSL_accept (in /usr/local/cyassl/lib/libcyassl.3.dylib)
==58560==    by 0x10000136E: server_test (in ./examples/server/.libs/server)
==58560==    by 0x10000219C: main (in ./examples/server/.libs/server)
==58560==  Uninitialised value was created by a heap allocation
==58560==    at 0x5237: malloc (in /usr/local/Cellar/valgrind/3.8.1/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==58560==    by 0x3A3A2: CyaSSL_Malloc (in /usr/local/cyassl/lib/libcyassl.3.dylib)
==58560==    by 0x1A898: CyaSSL_new (in /usr/local/cyassl/lib/libcyassl.3.dylib)
==58560==    by 0x1000012D0: server_test (in ./examples/server/.libs/server)
==58560==    by 0x10000219C: main (in ./examples/server/.libs/server)

the actual value being read uninitialized is: ssl->specs.kea

this also can be reproduced on commit 407397e8 (this is the closest one to 2.3.0 i can use due to other issues i've had).

thank you smile

tested this on 2.3.0 release now, it doesn't segfault, but complains nevertheless

AesDecrypt encountered improper key, set it up
AesDecrypt encountered improper key, set it up
AesDecrypt encountered improper key, set it up

ok, so i've investigated a bit. and looks like keys are just not being imported when session is not found for resume. so ssl->decrypt.aes = NULL and bad things happen.

i've been able to fix it with the following patch. Again, not sure if it would break something else. smile

It's probably the really wrong way to fix it, since resume relies on some state being present, but for what it's worth anyway.

diff --git a/src/internal.c b/src/internal.c
index 339b92d..a3966ac 100644
--- a/src/internal.c
+++ b/src/internal.c
@@ -7092,7 +7092,9 @@ int SetCipherList(Suites* s, const char* list)
             if (!session) {
                 ssl->options.resuming = 0;
                 CYASSL_MSG("Session lookup for resume failed");
+#if 0
                 break;   /* session lookup failed */
+#endif
             }
             if (MatchSuite(ssl, &clSuites) < 0) {
                 CYASSL_MSG("Unsupported cipher suite, ClientHello");

here's what i get from valgrind

accept state SERVER_HELLO_DONE
received record layer msg
got CHANGE CIPHER SPEC

==9094== Process terminating with default action of signal 10 (SIGBUS)
==9094==  Non-existent physical address at address 0x104
==9094==    at 0x6F06: memmove$VARIANT$sse3x (in /usr/local/Cellar/valgrind/3.8.1/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==9094==    by 0x33ABF: __inline_memcpy_chk (in /usr/local/cyassl/lib/libcyassl.3.dylib)
==9094==    by 0x3591F: AesCbcDecrypt (in /usr/local/cyassl/lib/libcyassl.3.dylib)
==9094==    by 0xE7FB: Decrypt (in /usr/local/cyassl/lib/libcyassl.3.dylib)
==9094==    by 0xE6B3: DecryptMessage (in /usr/local/cyassl/lib/libcyassl.3.dylib)
==9094==    by 0xF56C: ProcessReply (in /usr/local/cyassl/lib/libcyassl.3.dylib)
==9094==    by 0x1F172: CyaSSL_accept (in /usr/local/cyassl/lib/libcyassl.3.dylib)
==9094==    by 0x10000136E: server_test (in ./examples/server/.libs/server)
==9094==    by 0x10000219C: main (in ./examples/server/.libs/server)

Hey.
Found this today finally, seems like session resume corrupts memory when not found.

Context:
* latest github source
* macosx 10.7.4 (apple stock gcc 4.2) and SLES 11.1 (stock gcc 4.3.4)
* ./configure --prefix=/usr/local/cyassl --enable-hugecache --enable-fastmath --enable-debug
* start server

./examples/server/server -p 1105 -v 1

* execute command

echo -ne '160301006b010000670301506954bb10a154941e3c53b588a849080d9fb0886f94717ef2ae9c7c210b28482003d6b21d6d3428146bed287dba5e6e707dd5d88aff196f1cf5954633e4870cc0002000040005002f00330032000a00160013000900150012000300080014001100ff01001403010001011603010030ee9bca4c3dd96d96f718f79ead7ef7ac80057359b50cac8e48190e3afe09636dfe7a7f6f5d7c0ad72b4c9def13619612' | xxd -r -p | netcat localhost 1105 | xxd

here's debug log

<skipped all startup messages>

CyaSSL Entering SSL_set_fd
CyaSSL Leaving SSL_set_fd, return 1
CyaSSL Entering CyaSSL_SetTmpDH
CyaSSL Leaving CyaSSL_SetTmpDH, return 0
CyaSSL Entering SSL_accept()
growing input buffer

growing input buffer

received record layer msg
CyaSSL Entering DoHandShakeMsg()
CyaSSL Entering DoHandShakeMsgType
processing client hello
Client wants to resume session
Session lookup for resume failed
CyaSSL Entering MatchSuite
CyaSSL Entering VerifySuite
Requires RSA
Verified suite validity
CyaSSL Leaving DoHandShakeMsgType(), return 0
CyaSSL Leaving DoHandShakeMsg(), return 0
accept state ACCEPT_CLIENT_HELLO_DONE
accept state HELLO_VERIFY_SENT
accept state ACCEPT_FIRST_REPLY_DONE
growing output buffer

Shrinking output buffer

accept state SERVER_HELLO_SENT
growing output buffer

Shrinking output buffer

accept state CERT_SENT
accept state KEY_EXCHANGE_SENT
growing output buffer

Shrinking output buffer

accept state CERT_REQ_SENT
growing output buffer

Shrinking output buffer

accept state SERVER_HELLO_DONE
received record layer msg
got CHANGE CIPHER SPEC
Segmentation fault: 11

Help please smile

Awesome, thanks wink

> What are you working on if you don't mind me asking?
that's the frontend server for Badoo Mobile Application: http://badoo.com/en/mobile/iphone/

just noticed that wasn't actually the latest master i tested on.
so updated to "master f8b1066 fix sniffer cipher suite tests with user override"

the problem is still there for me.

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 sad

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 | xxd

i'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 tongue