You don't say what version of OpenSSL you were testing. It seems to be either 1.0.2 or 1.0.1 (not master). Anyway, comments inserted.
On Mon Dec 14 13:45:20 2015, skoripe...@juniper.net wrote: > Issue 1) > We could have failed to allocate the ctx->cipher_data in > EVP_CipherInit_ex > > ctx->cipher_data = OPENSSL_malloc(ctx->cipher->ctx_size); > if (!ctx->cipher_data) { > EVPerr(EVP_F_EVP_CIPHERINIT_EX, ERR_R_MALLOC_FAILURE); > return 0; > } > > We do subsequently return error from EVP_CipherInit_ex. However during > shutdown because of this error we are not checking for the NULL > cipher_data causing cores This seems very strange to me. If we have already returned a fatal error...then its just that - fatal. Don't try and do a graceful shutdown on an already dead connection. > ======================================================================================== > Issue 2 > In file pmeth_gn.c function EVP_PKEY_keygen, openssl code tries to > allocate EVP_PKEY using EVP_PKEY_new and immediately follows with a > dereference of the same in the below path without checking if the > allocation was successful or not. Fixed in 8e0a94a58. > Issue 3: > > In file s3_enc.c in function ssl3_digest_cached_records, > EVP_DigestInit_ex is called to initialize the EVP digest. Internally > to EVP_DigestInit_ex ctx->md_data is allocated and if it fails an > error is returned. However in ssl3_digest_cached_records the return > value is not checked, causing a null dereference with the below Fixed in ada5de7c and similar commit in master (this was the only one applicable to master BTW). > Issue 4: > In file ssl_lib.c, in function ssl_replace_hash, an EVP_MD_CTX is > created using EVP_MD_CTX_create. However, the return value of this > allocation is not checked and a dereference is made just below in > EVP_DigestInit_ex causing a core. This was fixed in 56d9134675 which was commited a few weeks before the date of your report. > ======================================================================================= > Issue 5: > In tl_enc.c, in function tls1_enc in the case of > /\* Explicit IV length, block ciphers and TLS version 1.1 or later \*/ > openssl tries to dereference cipher after getting the value of cipher > from s->enc_write_ctx. However cipher can be null. This can happen > because we returned NULL in Issue 4) above and s->enc_write_ctx- > >cipher might not have been set. Typically > s->enc_write_ctx->cipher would have been set in the below path but > because of Issue 4 above we did not set s->enc_write_ctx->cipher. Issue 4 above resulted in a core if it failed...so this confuses me! Anyway I could not see how this could occur if Issue 4 fails more gracefully. All callers of ssl_replace_hash propagate the error. Perhaps an issue similar to issue 1 above? Or maybe its been fixed since. I'm assuming this is no longer an issue. Please raise a new ticket if it is. > Issue 6: > Similar issue as above exists in s3_pkt.c function do_ssl3_write in > the case > /\* Explicit IV length, block ciphers and TLS version 1.1 or later \*/ > where again s->enc_write_ctx->cipher can be NULL. As for issue 5. > ======================================================================================= > Issue 7: > In file t1_enc.c, in function tls1_mac, openssl after calling > EVP_DigestSignFinal has an assert on the return value to be greater > than 0. However, EVP_DigestSignFinal internally allocates memory and > if this memory allocation fails, an error is returned. Hence this > assert is overaggressive for low memory cases. So Pls see if instead > of coring, the error can be handled gracefully. This was fixed in the same commit mentioned above that was committed a few weeks before your report. > ======================================================================================== > Issue 8: > In file t1_enc.c, in function tls1_setup_key_block, memory is > allocated twice for the keyblock through p1 and p2. If p1 succeeds but > p2 fails, p1 is freed but the freed pointer p1 is left dangling inside > s->s3->tmp.key_block which is later attempted to be freed while > freeing s->s3 resulting in a double free. > The fix would be to set the s->s3->tmp.key_block to NULL This was fixed in ec8f246e6ed4 a few weeks ago. Closing this ticket. Matt -- Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4180 Please log in as guest with password guest if prompted -- openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev