The following patches close several holes in openCryptoki's parsing of token data and correct NULL pointer deferences of the pValue attribue of some templates.
Signed-off-by: Kent Yoder <[email protected]> usr/lib/pkcs11/cca_stdll/h_extern.h | 13 ++++++++ usr/lib/pkcs11/cca_stdll/loadsave.c | 44 ++++++++++++++++++-------- usr/lib/pkcs11/cca_stdll/new_host.c | 9 +++-- usr/lib/pkcs11/common/h_extern.h | 18 +++++++++++ usr/lib/pkcs11/common/loadsave.c | 57 +++++++++++++++++++++++++--------- usr/lib/pkcs11/common/new_host.c | 8 +++- usr/lib/pkcs11/common/obj_mgr.c | 19 +++++++++-- usr/lib/pkcs11/common/object.c | 28 +++++++++++++++-- usr/lib/pkcs11/common/template.c | 32 +++++++++++++++--- usr/lib/pkcs11/tpm_stdll/h_extern.h | 14 ++++++++ usr/lib/pkcs11/tpm_stdll/loadsave.c | 58 +++++++++++++++++++++++++++-------- usr/lib/pkcs11/tpm_stdll/new_host.c | 4 +- 12 files changed, 244 insertions(+), 60 deletions(-) diff --git a/usr/lib/pkcs11/cca_stdll/h_extern.h b/usr/lib/pkcs11/cca_stdll/h_extern.h index 95af34c..e30f661 100644 --- a/usr/lib/pkcs11/cca_stdll/h_extern.h +++ b/usr/lib/pkcs11/cca_stdll/h_extern.h @@ -1662,6 +1662,8 @@ CK_BBOOL object_mgr_purge_private_token_objects( void ); CK_RV object_mgr_restore_obj( CK_BYTE *data, OBJECT *oldObj ); +CK_RV object_mgr_restore_obj_withSize( CK_BYTE *data, OBJECT *oldObj, int data_size ); + CK_RV object_mgr_set_attribute_values( SESSION * sess, CK_OBJECT_HANDLE handle, CK_ATTRIBUTE * pTemplate, @@ -1705,6 +1707,12 @@ CK_RV object_restore( CK_BYTE * data, OBJECT ** obj, CK_BBOOL replace ); +CK_RV object_restore_withSize( CK_BYTE * data, + OBJECT ** obj, + CK_BBOOL replace, + int data_size ); + + CK_RV object_set_attribute_values( OBJECT * obj, CK_ATTRIBUTE * pTemplate, CK_ULONG ulCount ); @@ -1779,6 +1787,11 @@ CK_RV template_unflatten( TEMPLATE ** tmpl, CK_BYTE * data, CK_ULONG count ); +CK_RV template_unflatten_withSize( TEMPLATE ** new_tmpl, + CK_BYTE * buf, + CK_ULONG count, + int buf_size ); + CK_RV template_validate_attribute( TEMPLATE * tmpl, CK_ATTRIBUTE * attr, CK_ULONG class, diff --git a/usr/lib/pkcs11/cca_stdll/loadsave.c b/usr/lib/pkcs11/cca_stdll/loadsave.c index 6feeb0c..ea6a816 100644 --- a/usr/lib/pkcs11/cca_stdll/loadsave.c +++ b/usr/lib/pkcs11/cca_stdll/loadsave.c @@ -28,6 +28,7 @@ #include <sys/ipc.h> #include <sys/file.h> #include <errno.h> +#include <syslog.h> #include <pwd.h> #include <grp.h> @@ -424,8 +425,9 @@ load_public_token_objects( void ) FILE *fp1 = NULL, *fp2 = NULL; CK_BYTE *buf = NULL; CK_BYTE tmp[PATH_MAX], fname[PATH_MAX],iname[PATH_MAX]; - CK_BBOOL priv; + CK_BBOOL priv = FALSE; CK_ULONG_32 size; + size_t read_size; sprintf((char *)iname,"%s/%s/%s",pk_dir,PK_LITE_OBJ_DIR, PK_LITE_OBJ_IDX); @@ -457,17 +459,25 @@ load_public_token_objects( void ) size = size -sizeof(CK_ULONG_32) - sizeof(CK_BBOOL); buf = (CK_BYTE *)malloc(size); if (!buf) { - fclose(fp1); fclose(fp2); - st_err_log(0, __FILE__, __LINE__); - return CKR_HOST_MEMORY; + LOG(LOG_ERR, "Cannot malloc %u bytes to read in token object %s (ignoring it)", + size, fname); + continue; } - fread( buf, size, 1, fp2 ); + read_size = fread( buf, 1, size, fp2 ); + if (read_size < size) { + LOG(LOG_ERR, "Cannot read in token object %s (ignoring it)", fname); + fclose(fp2); + free(buf); + continue; + } // ... grab object mutex here. MY_LockMutex(&obj_list_mutex); - object_mgr_restore_obj( buf, NULL ); + if (object_mgr_restore_obj_withSize(buf, NULL, size) != CKR_OK) { + LOG(LOG_ERR, "Cannot restore token object %s (ignoring it)", fname); + } MY_UnlockMutex(&obj_list_mutex); free( buf ); fclose( fp2 ); @@ -521,16 +531,17 @@ load_private_token_objects( void ) size = size - sizeof(CK_ULONG_32) - sizeof(CK_BBOOL); buf = (CK_BYTE *)malloc(size); if (!buf) { - st_err_log(0, __FILE__, __LINE__); - rc = CKR_HOST_MEMORY; - goto error; + fclose( fp2 ); + LOG(LOG_ERR, "Cannot malloc %u bytes to read in token object %s (ignoring it)", + size, fname); + continue; } rc = fread( (char *)buf, size, 1, fp2 ); if (rc != 1) { - st_err_log(4, __FILE__, __LINE__, __FUNCTION__); - rc = CKR_FUNCTION_FAILED; - goto error; + LOG(LOG_ERR, "Cannot read in token object %s (ignoring it)", fname); + fclose( fp2 ); + continue; } // Grab object list mutex @@ -636,6 +647,13 @@ restore_private_token_object( CK_BYTE * data, ptr = cleartxt; obj_data_len = *(CK_ULONG_32 *)ptr; + + if (obj_data_len > cleartxt_len) { + st_err_log(4, __FILE__, __LINE__, __FUNCTION__); + rc = CKR_FUNCTION_FAILED; + goto done; + } + ptr += sizeof(CK_ULONG_32); obj_data = ptr; @@ -1051,8 +1069,8 @@ reload_token_object( OBJECT *obj ) buf = (CK_BYTE *)malloc(size); if (!buf) { - st_err_log(0, __FILE__, __LINE__); rc = CKR_HOST_MEMORY; + LOG(LOG_ERR, "Cannot malloc %u bytes to read in token object %s (ignoring it)", size, fname); goto done; } diff --git a/usr/lib/pkcs11/cca_stdll/new_host.c b/usr/lib/pkcs11/cca_stdll/new_host.c index 93ee886..148febf 100644 --- a/usr/lib/pkcs11/cca_stdll/new_host.c +++ b/usr/lib/pkcs11/cca_stdll/new_host.c @@ -414,7 +414,6 @@ ST_Initialize(void **FunctionList, } } - // SAB XXX FIXME FIXME check return code... for all these... rc = load_token_data(); if (rc != CKR_OK) { *FunctionList = NULL; @@ -422,8 +421,10 @@ ST_Initialize(void **FunctionList, goto done; } - + /* no need to return error here, that would only prevent the stdll from loading. We load + * the token objects that we can and syslog the rest */ load_public_token_objects(); + XProcLock( xproclock ); global_shm->publ_loaded = TRUE; XProcUnLock( xproclock ); @@ -1424,7 +1425,9 @@ CK_RV SC_Login( ST_SESSION_HANDLE sSession, st_err_log(155, __FILE__, __LINE__); goto done; } - rc = load_private_token_objects(); + /* no need to return error here, that would only prevent the stdll from loading. We + * load the token objects that we can and syslog the rest */ + load_private_token_objects(); XProcLock( xproclock ); global_shm->priv_loaded = TRUE; diff --git a/usr/lib/pkcs11/common/h_extern.h b/usr/lib/pkcs11/common/h_extern.h index 5df755b..abb66ec 100755 --- a/usr/lib/pkcs11/common/h_extern.h +++ b/usr/lib/pkcs11/common/h_extern.h @@ -1937,6 +1937,8 @@ CK_BBOOL object_mgr_purge_private_token_objects( void ); CK_RV object_mgr_restore_obj( CK_BYTE *data, OBJECT *oldObj ); +CK_RV object_mgr_restore_obj_withSize( CK_BYTE *data, OBJECT *oldObj, int data_size ); + CK_RV object_mgr_set_attribute_values( SESSION * sess, CK_OBJECT_HANDLE handle, CK_ATTRIBUTE * pTemplate, @@ -1980,6 +1982,11 @@ CK_RV object_restore( CK_BYTE * data, OBJECT ** obj, CK_BBOOL replace ); +CK_RV object_restore_withSize( CK_BYTE * data, + OBJECT ** obj, + CK_BBOOL replace, + int data_size ); + CK_RV object_set_attribute_values( OBJECT * obj, CK_ATTRIBUTE * pTemplate, CK_ULONG ulCount ); @@ -2053,6 +2060,11 @@ CK_RV template_unflatten( TEMPLATE ** tmpl, CK_BYTE * data, CK_ULONG count ); +CK_RV template_unflatten_withSize( TEMPLATE ** new_tmpl, + CK_BYTE * buf, + CK_ULONG count, + int buf_size ); + CK_RV template_validate_attribute( TEMPLATE * tmpl, CK_ATTRIBUTE * attr, CK_ULONG class, @@ -2409,6 +2421,12 @@ extern token_spec_t token_specific; #define st_err_log(...) do { } while (0) #endif +#define LOG(priority, fmt, ...) \ + do { \ + openlog("openCryptoki", LOG_NDELAY|LOG_PID, LOG_USER); \ + syslog(priority, "%s " fmt, __FILE__, ##__VA_ARGS__); \ + } while (0) + /* CKA_HIDDEN will be used to filter return results on a C_FindObjects call. * Used for objects internal to a token for management of that token */ #define CKA_HIDDEN CKA_VENDOR_DEFINED + 0x01000000 diff --git a/usr/lib/pkcs11/common/loadsave.c b/usr/lib/pkcs11/common/loadsave.c index 64cee8c..d325894 100755 --- a/usr/lib/pkcs11/common/loadsave.c +++ b/usr/lib/pkcs11/common/loadsave.c @@ -307,6 +307,7 @@ #include <sys/ipc.h> #include <sys/file.h> #include <errno.h> +#include <syslog.h> #include <pwd.h> #include <grp.h> @@ -703,6 +704,7 @@ load_public_token_objects( void ) CK_BYTE tmp[PATH_MAX], fname[PATH_MAX], iname[PATH_MAX]; CK_BBOOL priv; CK_ULONG_32 size; + size_t read_size; sprintf((char *)iname,"%s/%s/%s",pk_dir,PK_LITE_OBJ_DIR, PK_LITE_OBJ_IDX); @@ -734,17 +736,25 @@ load_public_token_objects( void ) size = size -sizeof(CK_ULONG_32) - sizeof(CK_BBOOL); buf = (CK_BYTE *)malloc(size); if (!buf) { - fclose(fp1); fclose(fp2); - st_err_log(0, __FILE__, __LINE__); - return CKR_HOST_MEMORY; + LOG(LOG_ERR, "Cannot malloc %u bytes to read in token object %s (ignoring it)", + size, fname); + continue; } - fread( buf, size, 1, fp2 ); + read_size = fread( buf, 1, size, fp2 ); + if (read_size != size) { + fclose(fp2); + free(buf); + LOG(LOG_ERR, "Cannot read token object %s (ignoring it)", fname); + continue; + } // ... grab object mutex here. MY_LockMutex(&obj_list_mutex); - object_mgr_restore_obj( buf, NULL ); + if (object_mgr_restore_obj_withSize(buf, NULL, size) != CKR_OK) { + LOG(LOG_ERR, "Cannot restore token object %s (ignoring it)", fname); + } MY_UnlockMutex(&obj_list_mutex); free( buf ); fclose( fp2 ); @@ -767,6 +777,7 @@ load_private_token_objects( void ) CK_BBOOL priv; CK_ULONG_32 size; CK_RV rc; + size_t read_size; sprintf((char *)iname,"%s/%s/%s",pk_dir,PK_LITE_OBJ_DIR, PK_LITE_OBJ_IDX); @@ -798,16 +809,18 @@ load_private_token_objects( void ) size = size - sizeof(CK_ULONG_32) - sizeof(CK_BBOOL); buf = (CK_BYTE *)malloc(size); if (!buf) { - st_err_log(0, __FILE__, __LINE__); - rc = CKR_HOST_MEMORY; - goto error; + fclose( fp2 ); + LOG(LOG_ERR, "Cannot malloc %u bytes to read in token object %s (ignoring it)", + size, fname); + continue; } - rc = fread( (char *)buf, size, 1, fp2 ); - if (rc != 1) { - st_err_log(4, __FILE__, __LINE__, __FUNCTION__); - rc = CKR_FUNCTION_FAILED; - goto error; + read_size = fread( (char *)buf, 1, size, fp2 ); + if (read_size != size) { + free( buf ); + fclose( fp2 ); + LOG(LOG_ERR, "Cannot read token object %s (ignoring it)", fname); + continue; } // Grab object list mutex @@ -913,6 +926,14 @@ restore_private_token_object( CK_BYTE * data, ptr = cleartxt; obj_data_len = *(CK_ULONG_32 *)ptr; + + // prevent buffer overflow in sha_update + if (obj_data_len > cleartxt_len) { + st_err_log(4, __FILE__, __LINE__, __FUNCTION__); + rc = CKR_FUNCTION_FAILED; + goto done; + } + ptr += sizeof(CK_ULONG_32); obj_data = ptr; @@ -1312,6 +1333,7 @@ reload_token_object( OBJECT *obj ) CK_ULONG_32 size; CK_ULONG size_64; CK_RV rc; + size_t read_size; memset( (char *)fname, 0x0, sizeof(fname) ); @@ -1336,12 +1358,17 @@ reload_token_object( OBJECT *obj ) buf = (CK_BYTE *)malloc(size); if (!buf) { - st_err_log(0, __FILE__, __LINE__); rc = CKR_HOST_MEMORY; + LOG(LOG_ERR, "Cannot malloc %u bytes to read in token object %s (ignoring it)", size, fname); goto done; } - fread( buf, size, 1, fp ); + read_size = fread( buf, 1, size, fp ); + if (read_size != size) { + LOG(LOG_ERR, "Token object %s appears corrupted (ignoring it)", fname); + rc = CKR_FUNCTION_FAILED; + goto done; + } size_64 = size; diff --git a/usr/lib/pkcs11/common/new_host.c b/usr/lib/pkcs11/common/new_host.c index 96153f1..b6003e4 100755 --- a/usr/lib/pkcs11/common/new_host.c +++ b/usr/lib/pkcs11/common/new_host.c @@ -700,8 +700,9 @@ ST_Initialize(void **FunctionList, goto done; } - + /* no need to return error here, we load the token data we can and syslog the rest */ load_public_token_objects(); + XProcLock( xproclock ); global_shm->publ_loaded = TRUE; XProcUnLock( xproclock ); @@ -1702,7 +1703,10 @@ CK_RV SC_Login( ST_SESSION_HANDLE sSession, st_err_log(155, __FILE__, __LINE__); goto done; } - rc = load_private_token_objects(); + + /* no need to return error here, we load the token data we can and syslog the + * rest */ + load_private_token_objects(); XProcLock( xproclock ); global_shm->priv_loaded = TRUE; diff --git a/usr/lib/pkcs11/common/obj_mgr.c b/usr/lib/pkcs11/common/obj_mgr.c index f7eecbc..8fadbce 100755 --- a/usr/lib/pkcs11/common/obj_mgr.c +++ b/usr/lib/pkcs11/common/obj_mgr.c @@ -1530,6 +1530,11 @@ object_mgr_find_build_list( SESSION * sess, // that have the CKO_HW_FEATURE attribute set. - KEY if ((hw_feature == FALSE) && (template_attribute_find(obj->template, CKA_CLASS, &attr) == TRUE)) { + //axelrh: prevent segfault if pValue set to NULL (bad parse) + if (attr->pValue == NULL) { + st_err_log(3, __FILE__, __LINE__, __FUNCTION__); + return CKR_FUNCTION_FAILED; + } if (*(CK_OBJECT_CLASS *)attr->pValue == CKO_HW_FEATURE) goto next_loop; } @@ -1929,12 +1934,20 @@ object_mgr_purge_private_token_objects( void ) return TRUE; } - // // CK_RV object_mgr_restore_obj( CK_BYTE *data, OBJECT *oldObj ) { + return object_mgr_restore_obj_withSize(data, oldObj, -1); +} + +// +//Modified verrsion of object_mgr_restore_obj to bounds check +//If data_size==-1, won't check bounds +CK_RV +object_mgr_restore_obj_withSize( CK_BYTE *data, OBJECT *oldObj, int data_size ) +{ OBJECT * obj = NULL; CK_BBOOL priv; CK_RV rc; @@ -1953,10 +1966,10 @@ object_mgr_restore_obj( CK_BYTE *data, OBJECT *oldObj ) if (oldObj != NULL) { obj = oldObj; - rc = object_restore( data, &obj, TRUE ); + rc = object_restore_withSize( data, &obj, TRUE, data_size ); } else { - rc = object_restore( data, &obj, FALSE ); + rc = object_restore_withSize( data, &obj, FALSE, data_size ); if (rc == CKR_OK) { priv = object_is_private( obj ); diff --git a/usr/lib/pkcs11/common/object.c b/usr/lib/pkcs11/common/object.c index 03e3bfb..c73f22b 100755 --- a/usr/lib/pkcs11/common/object.c +++ b/usr/lib/pkcs11/common/object.c @@ -656,6 +656,10 @@ object_is_modifiable( OBJECT *obj ) if (found == FALSE) return TRUE; // should always be found but we default to TRUE + //axelrh: prevent dereferencing NULL from bad parse + if (attr->pValue == NULL) + return TRUE; //default to TRUE + modifiable = *(CK_BBOOL *)attr->pValue; return modifiable; @@ -680,7 +684,13 @@ object_is_private( OBJECT *obj ) if ( attr == NULL) return TRUE; - priv = *((CK_BBOOL *)attr->pValue); + //axelrh: prevent segfault caused by 0-len attribute + //that has a null pValue + CK_BBOOL *bboolPtr = (CK_BBOOL *)attr->pValue; + if (bboolPtr == NULL) + return TRUE; //default + + priv = *(bboolPtr); return priv; } @@ -715,6 +725,10 @@ object_is_token_object( OBJECT *obj ) if (found == FALSE) return FALSE; + //axelrh: prevent dereferencing NULL from bad parse + if (attr->pValue == NULL) + return FALSE; + is_token = *(CK_BBOOL *)attr->pValue; return is_token; } @@ -870,12 +884,20 @@ error: } - // // CK_RV object_restore( CK_BYTE *data, OBJECT **new_obj, CK_BBOOL replace ) { + return object_restore_withSize(data, new_obj, replace, -1); +} + +// +//Modified object_restore to prevent buffer overflow +//If data_size=-1, won't do bounds checking +CK_RV +object_restore_withSize( CK_BYTE *data, OBJECT **new_obj, CK_BBOOL replace, int data_size ) +{ TEMPLATE * tmpl = NULL; OBJECT * obj = NULL; CK_ULONG offset = 0; @@ -906,7 +928,7 @@ object_restore( CK_BYTE *data, OBJECT **new_obj, CK_BBOOL replace ) memcpy( &obj->name, data + offset, 8 ); offset += 8; - rc = template_unflatten( &tmpl, data + offset, count ); + rc = template_unflatten_withSize( &tmpl, data + offset, count, data_size ); if (rc != CKR_OK){ st_err_log(166, __FILE__, __LINE__); goto error; diff --git a/usr/lib/pkcs11/common/template.c b/usr/lib/pkcs11/common/template.c index c534ea7..42d7554 100755 --- a/usr/lib/pkcs11/common/template.c +++ b/usr/lib/pkcs11/common/template.c @@ -933,13 +933,25 @@ template_flatten( TEMPLATE * tmpl, return CKR_OK; } -// -// CK_RV template_unflatten( TEMPLATE ** new_tmpl, CK_BYTE * buf, CK_ULONG count ) { + return template_unflatten_withSize(new_tmpl, buf, count, -1); +} + + +// +//Modified version of template_unflatten that checks +//that buf isn't overread. buf_size=-1 turns off checking +//(for backwards compatability) +CK_RV +template_unflatten_withSize( TEMPLATE ** new_tmpl, + CK_BYTE * buf, + CK_ULONG count, + int buf_size ) +{ TEMPLATE * tmpl = NULL; CK_ATTRIBUTE * a2 = NULL; CK_BYTE * ptr = NULL; @@ -965,6 +977,10 @@ template_unflatten( TEMPLATE ** new_tmpl, ptr = buf; for (i=0; i < count; i++) { + if (buf_size >= 0 && ((ptr + sizeof(CK_ATTRIBUTE)) > (buf + buf_size))) { + return CKR_FUNCTION_FAILED; + } + if(long_len == 4) { a1_64 = (CK_ATTRIBUTE *)ptr; @@ -976,6 +992,10 @@ template_unflatten( TEMPLATE ** new_tmpl, return CKR_HOST_MEMORY; } + //if a buffer size is given, make sure it doesn't get overrun + if (buf_size >= 0 && (((void*)a1_64 + len) > ((void*)buf + buf_size))) { + return CKR_FUNCTION_FAILED; + } memcpy( a2, a1_64, len ); }else{ a1 = (CK_ATTRIBUTE_32 *)ptr; @@ -1036,10 +1056,12 @@ template_unflatten( TEMPLATE ** new_tmpl, pb2 += sizeof(CK_ATTRIBUTE); pb = (CK_BYTE *)a1; pb += sizeof(CK_ATTRIBUTE_32); + //if a buffer size is given, make sure it doesn't get overrun + if (buf_size >= 0 && (pb + a1->ulValueLen) > (buf + buf_size)) { + return CKR_FUNCTION_FAILED; + } memcpy( pb2, pb, a1->ulValueLen ); } - - } @@ -1058,8 +1080,6 @@ template_unflatten( TEMPLATE ** new_tmpl, ptr += len; else ptr += sizeof(CK_ATTRIBUTE_32) + a1->ulValueLen; - - } *new_tmpl = tmpl; diff --git a/usr/lib/pkcs11/tpm_stdll/h_extern.h b/usr/lib/pkcs11/tpm_stdll/h_extern.h index 448763a..d708373 100644 --- a/usr/lib/pkcs11/tpm_stdll/h_extern.h +++ b/usr/lib/pkcs11/tpm_stdll/h_extern.h @@ -1551,6 +1551,8 @@ CK_BBOOL object_mgr_purge_private_token_objects( void ); CK_RV object_mgr_restore_obj( CK_BYTE *data, OBJECT *oldObj ); +CK_RV object_mgr_restore_obj_withSize( CK_BYTE *data, OBJECT *oldObj, int data_size ); + CK_RV object_mgr_set_attribute_values( SESSION * sess, CK_OBJECT_HANDLE handle, CK_ATTRIBUTE * pTemplate, @@ -1594,6 +1596,11 @@ CK_RV object_restore( CK_BYTE * data, OBJECT ** obj, CK_BBOOL replace ); +CK_RV object_restore_withSize( CK_BYTE * data, + OBJECT ** obj, + CK_BBOOL replace, + int data_size ); + CK_RV object_set_attribute_values( OBJECT * obj, CK_ATTRIBUTE * pTemplate, CK_ULONG ulCount ); @@ -1665,6 +1672,12 @@ CK_RV template_unflatten( TEMPLATE ** tmpl, CK_BYTE * data, CK_ULONG count ); +CK_RV template_unflatten_withSize( TEMPLATE ** new_tmpl, + CK_BYTE * buf, + CK_ULONG count, + int buf_size ); + + CK_RV template_validate_attribute( TEMPLATE * tmpl, CK_ATTRIBUTE * attr, CK_ULONG class, @@ -2022,6 +2035,7 @@ extern token_spec_t token_specific; #define st_err_log(...) do { } while (0) #endif + /* custom attributes for the TPM token */ /* CKA_HIDDEN will be used to filter return results on a C_FindObjects call. Used * for objects internal to the TPM token for management */ diff --git a/usr/lib/pkcs11/tpm_stdll/loadsave.c b/usr/lib/pkcs11/tpm_stdll/loadsave.c index ceab8b1..7c23e2a 100644 --- a/usr/lib/pkcs11/tpm_stdll/loadsave.c +++ b/usr/lib/pkcs11/tpm_stdll/loadsave.c @@ -305,6 +305,7 @@ #include <sys/ipc.h> #include <sys/file.h> #include <errno.h> +#include <syslog.h> #include <pwd.h> #include <grp.h> @@ -319,6 +320,7 @@ //#include "args.h" #include "../api/apiproto.h" +#include "tpm_specific.h" #define MK_SIZE (AES_KEY_SIZE_256) @@ -791,6 +793,7 @@ load_public_token_objects( void ) CK_BBOOL priv; CK_ULONG_32 size; struct passwd *pw = NULL; + size_t buf_size; if ((pw = getpwuid(getuid())) == NULL){ LogError("getpwuid failed: %s", strerror(errno)); @@ -821,6 +824,7 @@ load_public_token_objects( void ) fread( &size, sizeof(CK_ULONG_32), 1, fp2 ); fread( &priv, sizeof(CK_BBOOL), 1, fp2 ); + if (priv == TRUE) { fclose( fp2 ); continue; @@ -830,17 +834,25 @@ load_public_token_objects( void ) size = size -sizeof(CK_ULONG_32) - sizeof(CK_BBOOL); buf = (CK_BYTE *)malloc(size); if (!buf) { - fclose(fp1); fclose(fp2); - st_err_log(0, __FILE__, __LINE__); - return CKR_HOST_MEMORY; + LOG(LOG_ERR, "Cannot malloc %u bytes to read in token object %s (ignoring it)", + size, fname); + continue; } - fread( buf, size, 1, fp2 ); + buf_size = fread( buf, 1, size, fp2 ); + if (buf_size != size) { + free(buf); + fclose(fp2); + LOG(LOG_ERR, "Cannot read in token object %s (ignoring it)", fname); + continue; + } // ... grab object mutex here. MY_LockMutex(&obj_list_mutex); - object_mgr_restore_obj( buf, NULL ); + if (object_mgr_restore_obj_withSize( buf, NULL, size ) != CKR_OK) { + LOG(LOG_ERR, "Cannot restore token object %s (ignoring it)", fname); + } MY_UnlockMutex(&obj_list_mutex); free( buf ); fclose( fp2 ); @@ -865,6 +877,7 @@ load_private_token_objects( void ) CK_ULONG_32 size; CK_RV rc; struct passwd *pw = NULL; + size_t buf_size; if ((pw = getpwuid(getuid())) == NULL){ LogError("getpwuid failed: %s", strerror(errno)); @@ -904,16 +917,18 @@ load_private_token_objects( void ) size = size - sizeof(CK_ULONG_32) - sizeof(CK_BBOOL); buf = (CK_BYTE *)malloc(size); if (!buf) { - st_err_log(0, __FILE__, __LINE__); - rc = CKR_HOST_MEMORY; - goto error; + fclose(fp2); + LOG(LOG_ERR, "Cannot malloc %u bytes to read in token object %s (ignoring it)", + size, fname); + continue; } rc = fread( (char *)buf, size, 1, fp2 ); if (rc != 1) { - st_err_log(4, __FILE__, __LINE__, __FUNCTION__); - rc = CKR_FUNCTION_FAILED; - goto error; + free(buf); + fclose(fp2); + LOG(LOG_ERR, "Cannot read in token object %s (ignoring it)", fname); + continue; } // Grab object list mutex @@ -1014,6 +1029,14 @@ restore_private_token_object( CK_BYTE * data, ptr = cleartxt; obj_data_len = *(CK_ULONG_32 *)ptr; + + //axelrh: prevent buffer overrun in sha_update + if (obj_data_len > cleartxt_len) { + st_err_log(4, __FILE__, __LINE__, __FUNCTION__); + rc = CKR_FUNCTION_FAILED; + goto done; + } + ptr += sizeof(CK_ULONG_32); obj_data = ptr; @@ -1380,6 +1403,7 @@ reload_token_object( OBJECT *obj ) CK_ULONG size_64; CK_RV rc; struct passwd *pw = NULL; + size_t read_size; if ((pw = getpwuid(getuid())) == NULL){ LogError("getpwuid failed: %s", strerror(errno)); @@ -1411,12 +1435,20 @@ reload_token_object( OBJECT *obj ) buf = (CK_BYTE *)malloc(size); if (!buf) { - st_err_log(0, __FILE__, __LINE__); + LOG(LOG_ERR, "Cannot malloc %u bytes to read in token object %s (ignoring it)", size, fname); rc = CKR_HOST_MEMORY; goto done; } - fread( buf, size, 1, fp ); + read_size = fread( buf, 1, size, fp ); + + //axelrh: make sure the buffer size read in is at least as big + //as the reported size of the token + if (read_size < size) { + LOG(LOG_ERR, "Token object %s appears corrupted (ignoring it)", fname); + rc = CKR_FUNCTION_FAILED; + goto done; + } size_64 = size; diff --git a/usr/lib/pkcs11/tpm_stdll/new_host.c b/usr/lib/pkcs11/tpm_stdll/new_host.c index 0933b3c..770de3c 100644 --- a/usr/lib/pkcs11/tpm_stdll/new_host.c +++ b/usr/lib/pkcs11/tpm_stdll/new_host.c @@ -484,7 +484,6 @@ CK_RV ST_Initialize( void **FunctionList, } } - // SAB XXX FIXME FIXME check return code... for all these... rc = load_token_data(); if (rc != CKR_OK) { *FunctionList = NULL; @@ -492,8 +491,9 @@ CK_RV ST_Initialize( void **FunctionList, goto done; } - + /* no need to check for error here, we load what we can and syslog the rest */ load_public_token_objects(); + XProcLock( xproclock ); global_shm->publ_loaded = TRUE; XProcUnLock( xproclock ); ------------------------------------------------------------------------------ This SF.net Dev2Dev email is sponsored by: Show off your parallel programming skills. Enter the Intel(R) Threading Challenge 2010. http://p.sf.net/sfu/intel-thread-sfd _______________________________________________ Opencryptoki-tech mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opencryptoki-tech
