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.

Submitted by: Götz Babin-Ebell <babinebell@trustcenter.de>
Reviewed by: Geoff Thorpe
PR: 98

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 <babinebell@trustcenter.de>]
+
   *) Improve diagnostics in file reading and command-line digests.
      [Ben Laurie aided and abetted by Solar Designer <solar@openwall.com>]
 

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))
 			{

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;
 		}

crypto/engine/hw_aep.c

@@ -71,6 +71,7 @@ typedef int pid_t;
 #include <openssl/crypto.h>
 #include <openssl/dso.h>
 #include <openssl/engine.h>
+#include <openssl/buffer.h>
 
 #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;
 		}

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;
 		}

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;
 		}

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;

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;
 		}

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;
 		}