Compiling OpenSSL code with MSVC and /W4 results in a number of warnings.
One category of warnings is particularly interesting - C4701 (potentially
uninitialized local variable 'name' used). This warning pretty much means
that there's a code path which results in uninitialized variables being used
or returned. Depending on compiler, its options, OS, values in registers
and/or stack, the results can be nondeterministic. Cases like this are very
hard to debug so it's rational to fix these issues.
This patch contains a set of trivial fixes for all the C4701 warnings (just
initializing variables to 0 or NULL or appropriate error code) to make sure
that deterministic values will be returned from all the execution paths.
RT#3835
Signed-off-by: Matt Caswell <matt@openssl.org>
Matt's note: All of these appear to be bogus warnings, i.e. there isn't
actually a code path where an unitialised variable could be used - its just
that the compiler hasn't been able to figure that out from the logic. So
this commit is just about silencing spurious warnings.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Just as with the OPENSSL_malloc calls, consistently use sizeof(*ptr)
for memset and memcpy. Remove needless casts for those functions.
For memset, replace alternative forms of zero with 0.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reusing an SSL object when it has encountered a fatal error can
have bad consequences. This is a bug in application code not libssl
but libssl should be more forgiving and not crash.
Reviewed-by: Rich Salz <rsalz@openssl.org>
For a local variable:
TYPE *p;
Allocations like this are "risky":
p = OPENSSL_malloc(sizeof(TYPE));
if the type of p changes, and the malloc call isn't updated, you
could get memory corruption. Instead do this:
p = OPENSSL_malloc(sizeof(*p));
Also fixed a few memset() calls that I noticed while doing this.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Incorrect name used for SSL_AD_INTERNAL_ERROR.
Signed-off-by: mancha security <mancha1@zoho.com>
Signed-off-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
After the finale, the "real" final part. :) Do a recursive grep with
"-B1 -w [a-zA-Z0-9_]*_free" to see if any of the preceeding lines are
an "if NULL" check that can be removed.
Reviewed-by: Tim Hudson <tjh@openssl.org>
Don't check for NULL before calling free functions. This gets:
ERR_STATE_free
ENGINE_free
DSO_free
CMAC_CTX_free
COMP_CTX_free
CONF_free
NCONF_free NCONF_free_data _CONF_free_data
A sk_free use within OBJ_sigid_free
TS_TST_INFO_free (rest of TS_ API was okay)
Doc update for UI_free (all uses were fine)
X509V3_conf_free
X509V3_section_free
X509V3_string_free
Reviewed-by: Richard Levitte <levitte@openssl.org>
This gets BN_.*free:
BN_BLINDING_free BN_CTX_free BN_FLG_FREE BN_GENCB_free
BN_MONT_CTX_free BN_RECP_CTX_free BN_clear_free BN_free BUF_MEM_free
Also fix a call to DSA_SIG_free to ccgost engine and remove some #ifdef'd
dead code in engines/e_ubsec.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Sanity check the |len| parameter to ensure it is positive. Thanks to Kevin
Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for
reporting this issue.
Reviewed-by: Andy Polyakov <appro@openssl.org>
The return value is checked for 0. This is currently safe but we should
really check for <= 0 since -1 is frequently used for error conditions.
Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
Solutions) for reporting this issue.
Reviewed-by: Andy Polyakov <appro@openssl.org>
For SSLv3 the code assumes that |header_length| > |md_block_size|. Whilst
this is true for all SSLv3 ciphersuites, this fact is far from obvious by
looking at the code. If this were not the case then an integer overflow
would occur, leading to a subsequent buffer overflow. Therefore I have
added an explicit sanity check to ensure header_length is always valid.
Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
Solutions) for reporting this issue.
Reviewed-by: Andy Polyakov <appro@openssl.org>
The various implementations of EVP_CTRL_AEAD_TLS_AAD expect a buffer of at
least 13 bytes long. Add sanity checks to ensure that the length is at
least that. Also add a new constant (EVP_AEAD_TLS1_AAD_LEN) to evp.h to
represent this length. Thanks to Kevin Wojtysiak (Int3 Solutions) and
Paramjot Oberoi (Int3 Solutions) for reporting this issue.
Reviewed-by: Andy Polyakov <appro@openssl.org>
Add OPENSSL_clear_free which merges cleanse and free.
(Names was picked to be similar to BN_clear_free, etc.)
Removed OPENSSL_freeFunc macro.
Fixed the small simple ones that are left:
CRYPTO_free CRYPTO_free_locked OPENSSL_free_locked
Reviewed-by: Richard Levitte <levitte@openssl.org>
Don't check for NULL before calling a free routine. This gets X509_.*free:
x509_name_ex_free X509_policy_tree_free X509_VERIFY_PARAM_free
X509_STORE_free X509_STORE_CTX_free X509_PKEY_free
X509_OBJECT_free_contents X509_LOOKUP_free X509_INFO_free
Reviewed-by: Richard Levitte <levitte@openssl.org>
There were a set of includes in dtls1.h which are now redundant due to the
libssl opaque work. This commit removes those includes, which also has the
effect of resolving one issue preventing building on windows (i.e. the
include of winsock.h)
Reviewed-by: Andy Polyakov <appro@openssl.org>
This is merges the old "rsalz-monolith" branch over to master. The biggest
change is that option parsing switch from cascasding 'else if strcmp("-foo")'
to a utility routine and somethin akin to getopt. Also, an error in the
command line no longer prints the full summary; use -help (or --help :)
for that. There have been many other changes and code-cleanup, see
bullet list below.
Special thanks to Matt for the long and detailed code review.
TEMPORARY:
For now, comment out CRYPTO_mem_leaks() at end of main
Tickets closed:
RT3515: Use 3DES in pkcs12 if built with no-rc2
RT1766: s_client -reconnect and -starttls broke
RT2932: Catch write errors
RT2604: port should be 'unsigned short'
RT2983: total_bytes undeclared #ifdef RENEG
RT1523: Add -nocert to fix output in x509 app
RT3508: Remove unused variable introduced by b09eb24
RT3511: doc fix; req default serial is random
RT1325,2973: Add more extensions to c_rehash
RT2119,3407: Updated to dgst.pod
RT2379: Additional typo fix
RT2693: Extra include of string.h
RT2880: HFS is case-insensitive filenames
RT3246: req command prints version number wrong
Other changes; incompatibilities marked with *:
Add SCSV support
Add -misalign to speed command
Make dhparam, dsaparam, ecparam, x509 output C in proper style
Make some internal ocsp.c functions void
Only display cert usages with -help in verify
Use global bio_err, remove "BIO*err" parameter from functions
For filenames, - always means stdin (or stdout as appropriate)
Add aliases for -des/aes "wrap" ciphers.
*Remove support for IISSGC (server gated crypto)
*The undocumented OCSP -header flag is now "-header name=value"
*Documented the OCSP -header flag
Reviewed-by: Matt Caswell <matt@openssl.org>
Add tables to convert between SSL_CIPHER fields and indices for ciphers
and MACs.
Reorganise ssl_ciph.c to use tables to lookup values and load them.
New functions SSL_CIPHER_get_cipher_nid and SSL_CIPHER_get_digest_nid.
Add documentation.
Reviewed-by: Rich Salz <rsalz@openssl.org>
EAP-FAST session resumption relies on handshake message lookahead
to determine server intentions. Commits
980bc1ec61
and
7b3ba508af
removed the lookahead so broke session resumption.
This change partially reverts the commits and brings the lookahead back
in reduced capacity for TLS + EAP-FAST only. Since EAP-FAST does not
support regular session tickets, the lookahead now only checks for a
Finished message.
Regular handshakes are unaffected by this change.
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
A 0-length ciphers list is never permitted. The old code only used to
reject an empty ciphers list for connections with a session ID. It
would later error out on a NULL structure, so this change just moves
the alert closer to the problem source.
Reviewed-by: Rich Salz <rsalz@openssl.org>
If OpenSSL is configured with no-tlsext then ssl_get_prev_session can read
past the end of the ClientHello message if the session_id length in the
ClientHello is invalid. This should not cause any security issues since the
underlying buffer is 16k in size. It should never be possible to overrun by
that many bytes.
This is probably made redundant by the previous commit - but you can never be
too careful.
With thanks to Qinghao Tang for reporting this issue.
Reviewed-by: Rich Salz <rsalz@openssl.org>
The ClientHello processing is insufficiently rigorous in its checks to make
sure that we don't read past the end of the message. This does not have
security implications due to the size of the underlying buffer - but still
needs to be fixed.
With thanks to Qinghao Tang for reporting this issue.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Fix a "&" that should have been "!" when processing read_ahead.
RT#3793
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Complete reimplementation of d2i_SSL_SESSION and i2d_SSL_SESSION using
new ASN.1 code and eliminating use of old ASN.1 macros.
Reviewed-by: Rich Salz <rsalz@openssl.org>
This engine is for VMS only, and isn't really part of the core OpenSSL
but rather a side project of its own that just happens to have tagged
along for a long time. The reasons why it has remained within the
OpenSSL source are long lost in history, and there not being any real
reason for it to remain here, it's time for it to move out.
This side project will appear as a project in its own right, the
location of which will be announced later on.
Reviewed-by: Tim Hudson <tjh@openssl.org>
With no more symlinks, there's no need for those variables, or the links
target. This also goes for all install: and uninstall: targets that do
nothing but copy $(EXHEADER) files, since that's now taken care of by the
top Makefile.
Also, removed METHTEST from test/Makefile. It looks like an old test that's
forgotten...
Reviewed-by: Rich Salz <rsalz@openssl.org>
Rather than making include/openssl/foo.h a symlink to
crypto/foo/foo.h, this change moves the file to include/openssl/foo.h
once and for all.
Likewise, move crypto/foo/footest.c to test/footest.c, instead of
symlinking it there.
Originally-by: Geoff Thorpe <geoff@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
Fix up various things that were missed during the record layer work. All
instances where we are breaking the encapsulation rules.
Reviewed-by: Richard Levitte <levitte@openssl.org>
EVP_.*free; this gets:
EVP_CIPHER_CTX_free EVP_PKEY_CTX_free EVP_PKEY_asn1_free
EVP_PKEY_asn1_set_free EVP_PKEY_free EVP_PKEY_free_it
EVP_PKEY_meth_free; and also EVP_CIPHER_CTX_cleanup
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
