Skip to content

Commit

Permalink
Fix memory leaks
Browse files Browse the repository at this point in the history
  • Loading branch information
@ducnguyen-sb
ducnguyen-sb committed Oct 22, 2023
1 parent b4c7ca0 commit 5a49b6b
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 28 deletions.
11 changes: 6 additions & 5 deletions src/sig_stfl/sig_stfl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ typedef struct OQS_SIG_STFL_SECRET_KEY {
/* The (maximum) length, in bytes, of secret keys for this signature scheme. */
size_t length_secret_key;

/* The variant specific secret key data */
/* The variant specific secret key data, must be allocated at the initialization. */
void *secret_key_data;

/* mutual exclusion struct */
Expand All @@ -281,7 +281,7 @@ typedef struct OQS_SIG_STFL_SECRET_KEY {
* @param[out] sk_len length of private key as a byte stream
* @param[out] sk_buf_ptr pointer to private key data as a byte stream
* @returns length of key material data available
* Caller deletes the buffer if memory was allocated.
* Caller is responsible to **unallocate** the pointer to buffer `sk_buf_ptr`.
*/
OQS_STATUS (*serialize_key)(OQS_SIG_STFL_SECRET_KEY *sk, size_t *sk_len, uint8_t **sk_buf_ptr);

Expand All @@ -292,8 +292,8 @@ typedef struct OQS_SIG_STFL_SECRET_KEY {
* @param[in] key_len length of the returned byte string
* @param[in] sk_buf The secret key data to populate key object
* @param[in] context application specific data
* @returns status of the operation populated with key material none-zero length. Caller
* deletes the buffer. if sk_buf is NULL the function returns the length
* @returns status of the operation populated with key material none-zero length.
* Caller is responsible to **unallocate** the buffer `sk_buf`.
*/
OQS_STATUS (*deserialize_key)(OQS_SIG_STFL_SECRET_KEY *sk, const size_t sk_len, const uint8_t *sk_buf, void *context);

Expand Down Expand Up @@ -511,9 +511,10 @@ OQS_STATUS OQS_SIG_STFL_SECRET_KEY_unlock(OQS_SIG_STFL_SECRET_KEY *sk);
*/
void OQS_SIG_STFL_SECRET_KEY_SET_store_cb(OQS_SIG_STFL_SECRET_KEY *sk, secure_store_sk store_cb, void *context);

/* Serialize stateful secret key data into a byte string, return an allocated buffer. Users is responsible to unallocate the buffer `sk_buf`. */
OQS_API OQS_STATUS OQS_SECRET_KEY_STFL_serialize_key(OQS_SIG_STFL_SECRET_KEY *sk, size_t *sk_len, uint8_t **sk_buf);

/* Insert lms byte string in an LMS secret key object */
/* Insert stateful byte string into an secret key object. User is responsible to unallocate buffer `sk_buf`. */
OQS_API OQS_STATUS OQS_SECRET_KEY_STFL_deserialize_key(OQS_SIG_STFL_SECRET_KEY *sk, size_t key_len, const uint8_t *sk_buf, void *context);

#if defined(__cplusplus)
Expand Down
2 changes: 1 addition & 1 deletion src/sig_stfl/xmss/sig_stfl_xmss.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ OQS_API OQS_STATUS OQS_SIG_STFL_alg_xmssmt_sigs_total(unsigned long long *total,
/* Generic XMSS SECRET_KEY object initialization */
OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_XMSS_new(size_t length_secret_key);

/* Serialize XMSS secret key data into a byte string */
/* Serialize XMSS secret key data into a byte string, return an allocated buffer. Users have to unallocated the buffer. */
OQS_STATUS OQS_SECRET_KEY_XMSS_serialize_key(OQS_SIG_STFL_SECRET_KEY *sk, size_t *sk_len, uint8_t **sk_buf_ptr);

/* Deserialize XMSS byte string into an XMSS secret key data */
Expand Down
9 changes: 2 additions & 7 deletions src/sig_stfl/xmss/sig_stfl_xmss_secret_key_functions.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ OQS_SIG_STFL_SECRET_KEY *OQS_SECRET_KEY_XMSS_new(size_t length_secret_key) {
return sk;
}

/* Serialize XMSS secret key data into a byte string. */
/* Serialize XMSS secret key data into a byte string, return an allocated buffer. Users have to unallocated the buffer. */
OQS_STATUS OQS_SECRET_KEY_XMSS_serialize_key(OQS_SIG_STFL_SECRET_KEY *sk, size_t *sk_len, uint8_t **sk_buf_ptr) {
if (sk == NULL || sk_len == NULL || sk_buf_ptr == NULL) {
return OQS_ERROR;
Expand Down Expand Up @@ -99,12 +99,7 @@ OQS_STATUS OQS_SECRET_KEY_XMSS_deserialize_key(OQS_SIG_STFL_SECRET_KEY *sk, cons
return OQS_ERROR;
}

sk->secret_key_data = malloc(sk_len);
if (sk->secret_key_data == NULL) {
return OQS_ERROR;
}

memcpy(sk->secret_key_data, sk_buf, sk_len);
memcpy(sk->secret_key_data, sk_buf, sk->length_secret_key);
sk->context = context;

return OQS_SUCCESS;
Expand Down
30 changes: 15 additions & 15 deletions tests/test_sig_stfl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ static uint8_t message_2[] = "The quick brown fox jumped from the tree.";
/*
* Write stateful secret keys to disk.
*/
static OQS_STATUS test_save_secret_key(uint8_t *key_buf, size_t buf_len, void *context) {
static OQS_STATUS save_secret_key(uint8_t *key_buf, size_t buf_len, void *context) {
if (key_buf == NULL || buf_len == 0 || context == NULL) {
return OQS_ERROR;
}
Expand Down Expand Up @@ -420,7 +420,7 @@ static OQS_STATUS sig_stfl_test_correctness(const char *method_name, const char

/* set context and secure store callback */
context = strdup(((file_store)));
OQS_SIG_STFL_SECRET_KEY_SET_store_cb(secret_key, test_save_secret_key, (void *)context);
OQS_SIG_STFL_SECRET_KEY_SET_store_cb(secret_key, save_secret_key, (void *)context);

#if OQS_USE_PTHREADS_IN_TESTS
sk_lock = (pthread_mutex_t *)malloc(sizeof(pthread_mutex_t));
Expand Down Expand Up @@ -571,13 +571,13 @@ static OQS_STATUS sig_stfl_test_correctness(const char *method_name, const char
static OQS_STATUS sig_stfl_test_secret_key(const char *method_name, const char *katfile) {
OQS_STATUS rc = OQS_SUCCESS;
OQS_SIG_STFL_SECRET_KEY *sk = NULL;
OQS_SIG_STFL_SECRET_KEY *sk_frm_file = NULL;
OQS_SIG_STFL_SECRET_KEY *sk_from_file = NULL;
unsigned long long num_sig_left = 0, max_num_sigs = 0;
OQS_SIG_STFL *sig_obj = NULL;
uint8_t *public_key = NULL;
uint8_t *frm_file_sk_buf = NULL;
uint8_t *from_file_sk_buf = NULL;
uint8_t *to_file_sk_buf = NULL;
size_t frm_file_sk_len = 0;
size_t from_file_sk_len = 0;
size_t to_file_sk_len = 0;
char *context = NULL;
char *context_2 = NULL;
Expand Down Expand Up @@ -654,27 +654,27 @@ static OQS_STATUS sig_stfl_test_secret_key(const char *method_name, const char *
/* set context and secure store callback */
if (sk->set_scrt_key_store_cb) {
context = strdup(file_store_name);
sk->set_scrt_key_store_cb(sk, test_save_secret_key, (void *)context);
sk->set_scrt_key_store_cb(sk, save_secret_key, (void *)context);
}

/* read secret key from disk */
frm_file_sk_buf = malloc(to_file_sk_len);
if (oqs_fload("sk", file_store_name, frm_file_sk_buf, to_file_sk_len, &frm_file_sk_len) != OQS_SUCCESS) {
from_file_sk_buf = malloc(to_file_sk_len);
if (oqs_fload("sk", file_store_name, from_file_sk_buf, to_file_sk_len, &from_file_sk_len) != OQS_SUCCESS) {
goto err;
}
if (to_file_sk_len != frm_file_sk_len) {
if (to_file_sk_len != from_file_sk_len) {
fprintf(stderr, "ERROR: OQS_SECRET_KEY_new stored length not equal read length\n");
goto err;
}

sk_frm_file = OQS_SIG_STFL_SECRET_KEY_new(method_name);
if (sk_frm_file == NULL) {
sk_from_file = OQS_SIG_STFL_SECRET_KEY_new(method_name);
if (sk_from_file == NULL) {
fprintf(stderr, "ERROR: 2nd OQS_SECRET_KEY_new failed\n");
goto err;
}

context_2 = strdup(file_store_name);
rc = OQS_SECRET_KEY_STFL_deserialize_key(sk_frm_file, frm_file_sk_len, frm_file_sk_buf, (void *)context_2);
rc = OQS_SECRET_KEY_STFL_deserialize_key(sk_from_file, from_file_sk_len, from_file_sk_buf, (void *)context_2);

if (rc != OQS_SUCCESS) {
fprintf(stderr, "OQS restore %s from file failed.\n", method_name);
Expand All @@ -693,7 +693,7 @@ static OQS_STATUS sig_stfl_test_secret_key(const char *method_name, const char *

OQS_MEM_insecure_free(public_key);
OQS_MEM_secure_free(to_file_sk_buf, to_file_sk_len);
OQS_MEM_secure_free(frm_file_sk_buf, frm_file_sk_len);
OQS_MEM_secure_free(from_file_sk_buf, from_file_sk_len);
OQS_SIG_STFL_free(sig_obj);
OQS_MEM_insecure_free(context);
OQS_MEM_insecure_free(context_2);
Expand Down Expand Up @@ -774,7 +774,7 @@ static OQS_STATUS sig_stfl_test_sig_gen(const char *method_name) {
key_store_name = convert_method_name_to_file_name(method_name);
/* set context and secure store callback */
context = strdup(((key_store_name)));
OQS_SIG_STFL_SECRET_KEY_SET_store_cb(lock_test_sk, test_save_secret_key, (void *)context);
OQS_SIG_STFL_SECRET_KEY_SET_store_cb(lock_test_sk, save_secret_key, (void *)context);

/*
* Get max num signature and the amount remaining
Expand Down Expand Up @@ -933,7 +933,7 @@ static OQS_STATUS sig_stfl_test_secret_key_lock(const char *method_name, const c
/* set context and secure store callback */
if (lock_test_sk->set_scrt_key_store_cb) {
lock_test_context = convert_method_name_to_file_name(method_name);
lock_test_sk->set_scrt_key_store_cb(lock_test_sk, test_save_secret_key, (void *)lock_test_context);
lock_test_sk->set_scrt_key_store_cb(lock_test_sk, save_secret_key, (void *)lock_test_context);
}

return OQS_SUCCESS;
Expand Down

0 comments on commit 5a49b6b

Please sign in to comment.