From a6c6874a1a0e625985ee2708afc30b7be05bdacf Mon Sep 17 00:00:00 2001 From: Geoff Thorpe Date: Fri, 21 Jun 2002 02:38:08 +0000 Subject: [PATCH] Make sure any ENGINE control commands make local copies of string pointers passed to them whenever necessary. Otherwise it is possible the caller may have overwritten (or deallocated) the original string data when a later ENGINE operation tries to use the stored values. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Submitted by: Götz Babin-Ebell Reviewed by: Geoff Thorpe PR: 98 --- CHANGES | 6 ++++++ crypto/engine/eng_dyn.c | 24 +++++++++++++++++++----- crypto/engine/hw_4758_cca.c | 29 +++++++++++++++++++++++------ crypto/engine/hw_aep.c | 26 ++++++++++++++++++++++---- crypto/engine/hw_atalla.c | 27 ++++++++++++++++++++++----- crypto/engine/hw_cswift.c | 27 ++++++++++++++++++++++----- crypto/engine/hw_ncipher.c | 27 ++++++++++++++++++++++----- crypto/engine/hw_nuron.c | 27 ++++++++++++++++++++++----- crypto/engine/hw_ubsec.c | 26 ++++++++++++++++++++++---- 9 files changed, 180 insertions(+), 39 deletions(-) diff --git a/CHANGES b/CHANGES index 44a1ba54f7..b45fea657e 100644 --- a/CHANGES +++ b/CHANGES @@ -92,6 +92,12 @@ Changes between 0.9.6d and 0.9.7 [XX xxx 2002] + *) Make sure any ENGINE control commands make local copies of string + pointers passed to them whenever necessary. Otherwise it is possible + the caller may have overwritten (or deallocated) the original string + data when a later ENGINE operation tries to use the stored values. + [Götz Babin-Ebell ] + *) Improve diagnostics in file reading and command-line digests. [Ben Laurie aided and abetted by Solar Designer ] diff --git a/crypto/engine/eng_dyn.c b/crypto/engine/eng_dyn.c index 4fefcc0cae..4139a16e76 100644 --- a/crypto/engine/eng_dyn.c +++ b/crypto/engine/eng_dyn.c @@ -157,6 +157,10 @@ static void dynamic_data_ctx_free_func(void *parent, void *ptr, dynamic_data_ctx *ctx = (dynamic_data_ctx *)ptr; if(ctx->dynamic_dso) DSO_free(ctx->dynamic_dso); + if(ctx->DYNAMIC_LIBNAME) + OPENSSL_free((void*)ctx->DYNAMIC_LIBNAME); + if(ctx->engine_id) + OPENSSL_free((void*)ctx->engine_id); OPENSSL_free(ctx); } } @@ -169,7 +173,7 @@ static int dynamic_set_data_ctx(ENGINE *e, dynamic_data_ctx **ctx) { dynamic_data_ctx *c; c = OPENSSL_malloc(sizeof(dynamic_data_ctx)); - if(!ctx) + if(!c) { ENGINEerr(ENGINE_F_SET_DATA_CTX,ERR_R_MALLOC_FAILURE); return 0; @@ -310,8 +314,13 @@ static int dynamic_ctrl(ENGINE *e, int cmd, long i, void *p, void (*f)()) /* a NULL 'p' or a string of zero-length is the same thing */ if(p && (strlen((const char *)p) < 1)) p = NULL; - ctx->DYNAMIC_LIBNAME = (const char *)p; - return 1; + if(ctx->DYNAMIC_LIBNAME) + OPENSSL_free((void*)ctx->DYNAMIC_LIBNAME); + if(p) + ctx->DYNAMIC_LIBNAME = BUF_strdup(p); + else + ctx->DYNAMIC_LIBNAME = NULL; + return (ctx->DYNAMIC_LIBNAME ? 1 : 0); case DYNAMIC_CMD_NO_VCHECK: ctx->no_vcheck = ((i == 0) ? 0 : 1); return 1; @@ -319,8 +328,13 @@ static int dynamic_ctrl(ENGINE *e, int cmd, long i, void *p, void (*f)()) /* a NULL 'p' or a string of zero-length is the same thing */ if(p && (strlen((const char *)p) < 1)) p = NULL; - ctx->engine_id = (const char *)p; - return 1; + if(ctx->engine_id) + OPENSSL_free((void*)ctx->engine_id); + if(p) + ctx->engine_id = BUF_strdup(p); + else + ctx->engine_id = NULL; + return (ctx->engine_id ? 1 : 0); case DYNAMIC_CMD_LIST_ADD: if((i < 0) || (i > 2)) { diff --git a/crypto/engine/hw_4758_cca.c b/crypto/engine/hw_4758_cca.c index 77d3d2ffdf..1053c52082 100644 --- a/crypto/engine/hw_4758_cca.c +++ b/crypto/engine/hw_4758_cca.c @@ -124,8 +124,24 @@ static F_RANDOMNUMBERGENERATE randomNumberGenerate; /* static variables */ /*------------------*/ -static const char def_CCA4758_LIB_NAME[] = CCA_LIB_NAME; -static const char *CCA4758_LIB_NAME = def_CCA4758_LIB_NAME; +static const char *CCA4758_LIB_NAME = NULL; +static const char *get_CCA4758_LIB_NAME(void) + { + if(CCA4758_LIB_NAME) + return CCA4758_LIB_NAME; + return CCA_LIB_NAME; + } +static void free_CCA4758_LIB_NAME(void) + { + if(CCA4758_LIB_NAME) + OPENSSL_free((void*)CCA4758_LIB_NAME); + CCA4758_LIB_NAME = NULL; + } +static long set_CCA4758_LIB_NAME(const char *name) + { + free_CCA4758_LIB_NAME(); + return (((CCA4758_LIB_NAME = BUF_strdup(name)) != NULL) ? 1 : 0); + } #ifndef OPENSSL_NO_RSA static const char* n_keyRecordRead = CSNDKRR; static const char* n_digitalSignatureGenerate = CSNDDSG; @@ -232,6 +248,7 @@ void ENGINE_load_4758cca(void) static int ibm_4758_cca_destroy(ENGINE *e) { ERR_unload_CCA4758_strings(); + free_CCA4758_LIB_NAME(); return 1; } @@ -243,7 +260,7 @@ static int ibm_4758_cca_init(ENGINE *e) goto err; } - dso = DSO_load(NULL, CCA4758_LIB_NAME , NULL, 0); + dso = DSO_load(NULL, get_CCA4758_LIB_NAME(), NULL, 0); if(!dso) { CCA4758err(CCA4758_F_IBM_4758_CCA_INIT,CCA4758_R_DSO_FAILURE); @@ -299,7 +316,8 @@ err: static int ibm_4758_cca_finish(ENGINE *e) { - if(dso) + free_CCA4758_LIB_NAME(); + if(!dso) { CCA4758err(CCA4758_F_IBM_4758_CCA_FINISH, CCA4758_R_NOT_LOADED); @@ -340,8 +358,7 @@ static int ibm_4758_cca_ctrl(ENGINE *e, int cmd, long i, void *p, void (*f)()) CCA4758_R_ALREADY_LOADED); return 0; } - CCA4758_LIB_NAME = (const char *)p; - return 1; + return set_CCA4758_LIB_NAME((const char *)p); default: break; } diff --git a/crypto/engine/hw_aep.c b/crypto/engine/hw_aep.c index cf4507cff1..ab36d36a76 100644 --- a/crypto/engine/hw_aep.c +++ b/crypto/engine/hw_aep.c @@ -71,6 +71,7 @@ typedef int pid_t; #include #include #include +#include #ifndef OPENSSL_NO_HW #ifndef OPENSSL_NO_HW_AEP @@ -363,7 +364,24 @@ static DSO *aep_dso = NULL; /* These are the static string constants for the DSO file name and the function * symbol names to bind to. */ -static const char *AEP_LIBNAME = "aep"; +static const char *AEP_LIBNAME = NULL; +static const char *get_AEP_LIBNAME(void) + { + if(AEP_LIBNAME) + return AEP_LIBNAME; + return "aep"; + } +static void free_AEP_LIBNAME(void) + { + if(AEP_LIBNAME) + OPENSSL_free((void*)AEP_LIBNAME); + AEP_LIBNAME = NULL; + } +static long set_AEP_LIBNAME(const char *name) + { + free_AEP_LIBNAME(); + return ((AEP_LIBNAME = BUF_strdup(name)) != NULL ? 1 : 0); + } static const char *AEP_F1 = "AEP_ModExp"; static const char *AEP_F2 = "AEP_ModExpCrt"; @@ -412,7 +430,7 @@ static int aep_init(ENGINE *e) } /* Attempt to load libaep.so. */ - aep_dso = DSO_load(NULL, AEP_LIBNAME, NULL, 0); + aep_dso = DSO_load(NULL, get_AEP_LIBNAME(), NULL, 0); if(aep_dso == NULL) { @@ -474,6 +492,7 @@ static int aep_init(ENGINE *e) /* Destructor (complements the "ENGINE_aep()" constructor) */ static int aep_destroy(ENGINE *e) { + free_AEP_LIBNAME(); ERR_unload_AEPHK_strings(); return 1; } @@ -549,8 +568,7 @@ static int aep_ctrl(ENGINE *e, int cmd, long i, void *p, void (*f)()) AEPHK_R_ALREADY_LOADED); return 0; } - AEP_LIBNAME = (const char *)p; - return 1; + return set_AEP_LIBNAME((const char*)p); default: break; } diff --git a/crypto/engine/hw_atalla.c b/crypto/engine/hw_atalla.c index 696cfcf156..6151c46902 100644 --- a/crypto/engine/hw_atalla.c +++ b/crypto/engine/hw_atalla.c @@ -286,8 +286,24 @@ static tfnASI_GetPerformanceStatistics *p_Atalla_GetPerformanceStatistics = NULL * atasi.dll on win32). For the purposes of testing, I have created a symbollic * link called "libatasi.so" so that we can use native name-translation - a * better solution will be needed. */ -static const char def_ATALLA_LIBNAME[] = "atasi"; -static const char *ATALLA_LIBNAME = def_ATALLA_LIBNAME; +static const char *ATALLA_LIBNAME = NULL; +static const char *get_ATALLA_LIBNAME(void) + { + if(ATALLA_LIBNAME) + return ATALLA_LIBNAME; + return "atasi"; + } +static void free_ATALLA_LIBNAME(void) + { + if(ATALLA_LIBNAME) + OPENSSL_free((void*)ATALLA_LIBNAME); + ATALLA_LIBNAME = NULL; + } +static long set_ATALLA_LIBNAME(const char *name) + { + free_ATALLA_LIBNAME(); + return (((ATALLA_LIBNAME = BUF_strdup(name)) != NULL) ? 1 : 0); + } static const char *ATALLA_F1 = "ASI_GetHardwareConfig"; static const char *ATALLA_F2 = "ASI_RSAPrivateKeyOpFn"; static const char *ATALLA_F3 = "ASI_GetPerformanceStatistics"; @@ -295,6 +311,7 @@ static const char *ATALLA_F3 = "ASI_GetPerformanceStatistics"; /* Destructor (complements the "ENGINE_atalla()" constructor) */ static int atalla_destroy(ENGINE *e) { + free_ATALLA_LIBNAME(); /* Unload the atalla error strings so any error state including our * functs or reasons won't lead to a segfault (they simply get displayed * without corresponding string data because none will be found). */ @@ -324,7 +341,7 @@ static int atalla_init(ENGINE *e) * drivers really use - for now a symbollic link needs to be * created on the host system from libatasi.so to atasi.so on * unix variants. */ - atalla_dso = DSO_load(NULL, ATALLA_LIBNAME, NULL, 0); + atalla_dso = DSO_load(NULL, get_ATALLA_LIBNAME(), NULL, 0); if(atalla_dso == NULL) { ATALLAerr(ATALLA_F_ATALLA_INIT,ATALLA_R_NOT_LOADED); @@ -364,6 +381,7 @@ err: static int atalla_finish(ENGINE *e) { + free_ATALLA_LIBNAME(); if(atalla_dso == NULL) { ATALLAerr(ATALLA_F_ATALLA_FINISH,ATALLA_R_NOT_LOADED); @@ -397,8 +415,7 @@ static int atalla_ctrl(ENGINE *e, int cmd, long i, void *p, void (*f)()) ATALLAerr(ATALLA_F_ATALLA_CTRL,ATALLA_R_ALREADY_LOADED); return 0; } - ATALLA_LIBNAME = (const char *)p; - return 1; + return set_ATALLA_LIBNAME((const char *)p); default: break; } diff --git a/crypto/engine/hw_cswift.c b/crypto/engine/hw_cswift.c index d8b380550f..31a79a9d16 100644 --- a/crypto/engine/hw_cswift.c +++ b/crypto/engine/hw_cswift.c @@ -280,8 +280,24 @@ t_swSimpleRequest *p_CSwift_SimpleRequest = NULL; t_swReleaseAccContext *p_CSwift_ReleaseAccContext = NULL; /* Used in the DSO operations. */ -static const char def_CSWIFT_LIBNAME[] = "swift"; -static const char *CSWIFT_LIBNAME = def_CSWIFT_LIBNAME; +static const char *CSWIFT_LIBNAME = NULL; +static const char *get_CSWIFT_LIBNAME(void) + { + if(CSWIFT_LIBNAME) + return CSWIFT_LIBNAME; + return "swift"; + } +static void free_CSWIFT_LIBNAME(void) + { + if(CSWIFT_LIBNAME) + OPENSSL_free((void*)CSWIFT_LIBNAME); + CSWIFT_LIBNAME = NULL; + } +static long set_CSWIFT_LIBNAME(const char *name) + { + free_CSWIFT_LIBNAME(); + return (((CSWIFT_LIBNAME = BUF_strdup(name)) != NULL) ? 1 : 0); + } static const char *CSWIFT_F1 = "swAcquireAccContext"; static const char *CSWIFT_F2 = "swAttachKeyParam"; static const char *CSWIFT_F3 = "swSimpleRequest"; @@ -313,6 +329,7 @@ static void release_context(SW_CONTEXT_HANDLE hac) /* Destructor (complements the "ENGINE_cswift()" constructor) */ static int cswift_destroy(ENGINE *e) { + free_CSWIFT_LIBNAME(); ERR_unload_CSWIFT_strings(); return 1; } @@ -332,7 +349,7 @@ static int cswift_init(ENGINE *e) goto err; } /* Attempt to load libswift.so/swift.dll/whatever. */ - cswift_dso = DSO_load(NULL, CSWIFT_LIBNAME, NULL, 0); + cswift_dso = DSO_load(NULL, get_CSWIFT_LIBNAME(), NULL, 0); if(cswift_dso == NULL) { CSWIFTerr(CSWIFT_F_CSWIFT_INIT,CSWIFT_R_NOT_LOADED); @@ -377,6 +394,7 @@ err: static int cswift_finish(ENGINE *e) { + free_CSWIFT_LIBNAME(); if(cswift_dso == NULL) { CSWIFTerr(CSWIFT_F_CSWIFT_FINISH,CSWIFT_R_NOT_LOADED); @@ -411,8 +429,7 @@ static int cswift_ctrl(ENGINE *e, int cmd, long i, void *p, void (*f)()) CSWIFTerr(CSWIFT_F_CSWIFT_CTRL,CSWIFT_R_ALREADY_LOADED); return 0; } - CSWIFT_LIBNAME = (const char *)p; - return 1; + return set_CSWIFT_LIBNAME((const char *)p); default: break; } diff --git a/crypto/engine/hw_ncipher.c b/crypto/engine/hw_ncipher.c index 4762a54e3d..7c1141afa9 100644 --- a/crypto/engine/hw_ncipher.c +++ b/crypto/engine/hw_ncipher.c @@ -422,8 +422,24 @@ static HWCryptoHook_RSAUnloadKey_t *p_hwcrhk_RSAUnloadKey = NULL; static HWCryptoHook_ModExpCRT_t *p_hwcrhk_ModExpCRT = NULL; /* Used in the DSO operations. */ -static const char def_HWCRHK_LIBNAME[] = "nfhwcrhk"; -static const char *HWCRHK_LIBNAME = def_HWCRHK_LIBNAME; +static const char *HWCRHK_LIBNAME = NULL; +static void free_HWCRHK_LIBNAME(void) + { + if(HWCRHK_LIBNAME) + OPENSSL_free((void*)HWCRHK_LIBNAME); + HWCRHK_LIBNAME = NULL; + } +static const char *get_HWCRHK_LIBNAME(void) + { + if(HWCRHK_LIBNAME) + return HWCRHK_LIBNAME; + return "nfhwcrhk"; + } +static long set_HWCRHK_LIBNAME(const char *name) + { + free_HWCRHK_LIBNAME(); + return (((HWCRHK_LIBNAME = BUF_strdup(name)) != NULL) ? 1 : 0); + } static const char *n_hwcrhk_Init = "HWCryptoHook_Init"; static const char *n_hwcrhk_Finish = "HWCryptoHook_Finish"; static const char *n_hwcrhk_ModExp = "HWCryptoHook_ModExp"; @@ -469,6 +485,7 @@ static void release_context(HWCryptoHook_ContextHandle hac) /* Destructor (complements the "ENGINE_ncipher()" constructor) */ static int hwcrhk_destroy(ENGINE *e) { + free_HWCRHK_LIBNAME(); ERR_unload_HWCRHK_strings(); return 1; } @@ -494,7 +511,7 @@ static int hwcrhk_init(ENGINE *e) goto err; } /* Attempt to load libnfhwcrhk.so/nfhwcrhk.dll/whatever. */ - hwcrhk_dso = DSO_load(NULL, HWCRHK_LIBNAME, NULL, 0); + hwcrhk_dso = DSO_load(NULL, get_HWCRHK_LIBNAME(), NULL, 0); if(hwcrhk_dso == NULL) { HWCRHKerr(HWCRHK_F_HWCRHK_INIT,HWCRHK_R_DSO_FAILURE); @@ -586,6 +603,7 @@ err: static int hwcrhk_finish(ENGINE *e) { int to_return = 1; + free_HWCRHK_LIBNAME(); if(hwcrhk_dso == NULL) { HWCRHKerr(HWCRHK_F_HWCRHK_FINISH,HWCRHK_R_NOT_LOADED); @@ -634,8 +652,7 @@ static int hwcrhk_ctrl(ENGINE *e, int cmd, long i, void *p, void (*f)()) HWCRHKerr(HWCRHK_F_HWCRHK_CTRL,ERR_R_PASSED_NULL_PARAMETER); return 0; } - HWCRHK_LIBNAME = (const char *)p; - return 1; + return set_HWCRHK_LIBNAME((const char *)p); case ENGINE_CTRL_SET_LOGSTREAM: { BIO *bio = (BIO *)p; diff --git a/crypto/engine/hw_nuron.c b/crypto/engine/hw_nuron.c index 2672012154..130b6d8b40 100644 --- a/crypto/engine/hw_nuron.c +++ b/crypto/engine/hw_nuron.c @@ -69,8 +69,24 @@ #define NURON_LIB_NAME "nuron engine" #include "hw_nuron_err.c" -static const char def_NURON_LIBNAME[] = "nuronssl"; -static const char *NURON_LIBNAME = def_NURON_LIBNAME; +static const char *NURON_LIBNAME = NULL; +static const char *get_NURON_LIBNAME(void) + { + if(NURON_LIBNAME) + return NURON_LIBNAME; + return "nuronssl"; + } +static void free_NURON_LIBNAME(void) + { + if(NURON_LIBNAME) + OPENSSL_free((void*)NURON_LIBNAME); + NURON_LIBNAME = NULL; + } +static long set_NURON_LIBNAME(const char *name) + { + free_NURON_LIBNAME(); + return (((NURON_LIBNAME = BUF_strdup(name)) != NULL) ? 1 : 0); + } static const char *NURON_F1 = "nuron_mod_exp"; /* The definitions for control commands specific to this engine */ @@ -90,6 +106,7 @@ static DSO *pvDSOHandle = NULL; static int nuron_destroy(ENGINE *e) { + free_NURON_LIBNAME(); ERR_unload_NURON_strings(); return 1; } @@ -102,7 +119,7 @@ static int nuron_init(ENGINE *e) return 0; } - pvDSOHandle = DSO_load(NULL, NURON_LIBNAME, NULL, + pvDSOHandle = DSO_load(NULL, get_NURON_LIBNAME(), NULL, DSO_FLAG_NAME_TRANSLATION_EXT_ONLY); if(!pvDSOHandle) { @@ -122,6 +139,7 @@ static int nuron_init(ENGINE *e) static int nuron_finish(ENGINE *e) { + free_NURON_LIBNAME(); if(pvDSOHandle == NULL) { NURONerr(NURON_F_NURON_FINISH,NURON_R_NOT_LOADED); @@ -153,8 +171,7 @@ static int nuron_ctrl(ENGINE *e, int cmd, long i, void *p, void (*f)()) NURONerr(NURON_F_NURON_CTRL,NURON_R_ALREADY_LOADED); return 0; } - NURON_LIBNAME = (const char *)p; - return 1; + return set_NURON_LIBNAME((const char *)p); default: break; } diff --git a/crypto/engine/hw_ubsec.c b/crypto/engine/hw_ubsec.c index 743c06043c..63397f868c 100644 --- a/crypto/engine/hw_ubsec.c +++ b/crypto/engine/hw_ubsec.c @@ -304,7 +304,24 @@ static int max_key_len = 1024; /* ??? */ * symbol names to bind to. */ -static const char *UBSEC_LIBNAME = "ubsec"; +static const char *UBSEC_LIBNAME = NULL; +static const char *get_UBSEC_LIBNAME(void) + { + if(UBSEC_LIBNAME) + return UBSEC_LIBNAME; + return "ubsec"; + } +static void free_UBSEC_LIBNAME(void) + { + if(UBSEC_LIBNAME) + OPENSSL_free((void*)UBSEC_LIBNAME); + UBSEC_LIBNAME = NULL; + } +static long set_UBSEC_LIBNAME(const char *name) + { + free_UBSEC_LIBNAME(); + return (((UBSEC_LIBNAME = BUF_strdup(name)) != NULL) ? 1 : 0); + } static const char *UBSEC_F1 = "ubsec_bytes_to_bits"; static const char *UBSEC_F2 = "ubsec_bits_to_bytes"; static const char *UBSEC_F3 = "ubsec_open"; @@ -328,6 +345,7 @@ static const char *UBSEC_F13 = "ubsec_max_key_len_ioctl"; /* Destructor (complements the "ENGINE_ubsec()" constructor) */ static int ubsec_destroy(ENGINE *e) { + free_UBSEC_LIBNAME(); ERR_unload_UBSEC_strings(); return 1; } @@ -364,7 +382,7 @@ static int ubsec_init(ENGINE *e) /* * Attempt to load libubsec.so/ubsec.dll/whatever. */ - ubsec_dso = DSO_load(NULL, UBSEC_LIBNAME, NULL, 0); + ubsec_dso = DSO_load(NULL, get_UBSEC_LIBNAME(), NULL, 0); if(ubsec_dso == NULL) { UBSECerr(UBSEC_F_UBSEC_INIT, UBSEC_R_DSO_FAILURE); @@ -459,6 +477,7 @@ err: static int ubsec_finish(ENGINE *e) { + free_UBSEC_LIBNAME(); if(ubsec_dso == NULL) { UBSECerr(UBSEC_F_UBSEC_FINISH, UBSEC_R_NOT_LOADED); @@ -508,8 +527,7 @@ static int ubsec_ctrl(ENGINE *e, int cmd, long i, void *p, void (*f)()) UBSECerr(UBSEC_F_UBSEC_CTRL,UBSEC_R_ALREADY_LOADED); return 0; } - UBSEC_LIBNAME = (const char *)p; - return 1; + return set_UBSEC_LIBNAME((const char *)p); default: break; } -- 2.25.1