Make sure any ENGINE control commands make local copies of string
authorGeoff Thorpe <geoff@openssl.org>
Fri, 21 Jun 2002 02:38:08 +0000 (02:38 +0000)
committerGeoff Thorpe <geoff@openssl.org>
Fri, 21 Jun 2002 02:38:08 +0000 (02:38 +0000)
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
crypto/engine/eng_dyn.c
crypto/engine/hw_4758_cca.c
crypto/engine/hw_aep.c
crypto/engine/hw_atalla.c
crypto/engine/hw_cswift.c
crypto/engine/hw_ncipher.c
crypto/engine/hw_nuron.c
crypto/engine/hw_ubsec.c

diff --git a/CHANGES b/CHANGES
index 44a1ba5..b45fea6 100644 (file)
--- a/CHANGES
+++ b/CHANGES
  
  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>]
 
index 4fefcc0..4139a16 100644 (file)
@@ -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))
                        {
index 77d3d2f..1053c52 100644 (file)
@@ -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;
                }
index cf4507c..ab36d36 100644 (file)
@@ -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;
                }
index 696cfcf..6151c46 100644 (file)
@@ -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;
                }
index d8b3805..31a79a9 100644 (file)
@@ -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;
                }
index 4762a54..7c1141a 100644 (file)
@@ -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;
index 2672012..130b6d8 100644 (file)
 #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;
                }
index 743c060..63397f8 100644 (file)
@@ -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;
                }