From b7ec10d4618ac74d7ed45127254c5e1e1a1d8472 Mon Sep 17 00:00:00 2001 From: Aditya Patwardhan Date: Mon, 3 Oct 2022 09:02:04 +0530 Subject: [PATCH] protocommm/esp_srp: Fix small issues reported by coverity. --- components/protocomm/src/crypto/srp6a/esp_srp.c | 3 ++- components/protocomm/src/crypto/srp6a/esp_srp_mpi.c | 3 +++ .../protocomm/src/crypto/srp6a/include/esp_srp.h | 2 +- components/protocomm/src/security/security2.c | 12 +++++++++--- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/components/protocomm/src/crypto/srp6a/esp_srp.c b/components/protocomm/src/crypto/srp6a/esp_srp.c index 79159bc41f..36fbc08342 100644 --- a/components/protocomm/src/crypto/srp6a/esp_srp.c +++ b/components/protocomm/src/crypto/srp6a/esp_srp.c @@ -388,7 +388,7 @@ error: return ESP_FAIL; } -esp_err_t esp_srp_get_session_key(esp_srp_handle_t *hd, char *bytes_A, int len_A, char **bytes_key, int *len_key) +esp_err_t esp_srp_get_session_key(esp_srp_handle_t *hd, char *bytes_A, int len_A, char **bytes_key, uint16_t *len_key) { esp_mpi_t *u = NULL; esp_mpi_t *vu = NULL; @@ -524,6 +524,7 @@ esp_err_t esp_srp_exchange_proofs(esp_srp_handle_t *hd, char *username, uint16_t ESP_LOG_BUFFER_HEX_LEVEL(TAG, (char *)digest, sizeof(digest), ESP_LOG_DEBUG); if (memcmp(bytes_user_proof, digest, SHA512_HASH_SZ) != 0) { + free(s); return ESP_FAIL; } diff --git a/components/protocomm/src/crypto/srp6a/esp_srp_mpi.c b/components/protocomm/src/crypto/srp6a/esp_srp_mpi.c index c02cbab000..2cb60e4ee9 100644 --- a/components/protocomm/src/crypto/srp6a/esp_srp_mpi.c +++ b/components/protocomm/src/crypto/srp6a/esp_srp_mpi.c @@ -26,6 +26,7 @@ esp_mpi_t *esp_mpi_new_from_hex(const char *hex) int ret = mbedtls_mpi_read_string(a, 16, hex); if (ret != 0) { printf("mbedtls_mpi_read_string() failed, returned %x\n", ret); + esp_mpi_free(a); return NULL; } return a; @@ -41,6 +42,7 @@ esp_mpi_t *esp_mpi_new_from_bin(const char *str, int str_len) int ret = mbedtls_mpi_read_binary(a, (unsigned char *)str, str_len); if (ret != 0) { printf("mbedtls_mpi_read_binary() failed, returned %x\n", ret); + esp_mpi_free(a); return NULL; } return a; @@ -81,6 +83,7 @@ char *esp_mpi_to_bin(esp_mpi_t *bn, int *len) int ret = mbedtls_mpi_write_binary(bn, (unsigned char *)p, *len); if (ret != 0) { printf("mbedtls_mpi_read_string() failed, returned %x\n", ret); + free(p); return NULL; } return p; diff --git a/components/protocomm/src/crypto/srp6a/include/esp_srp.h b/components/protocomm/src/crypto/srp6a/include/esp_srp.h index 388fab7acb..972704f038 100644 --- a/components/protocomm/src/crypto/srp6a/include/esp_srp.h +++ b/components/protocomm/src/crypto/srp6a/include/esp_srp.h @@ -88,7 +88,7 @@ esp_err_t esp_srp_srv_pubkey_from_salt_verifier(esp_srp_handle_t *hd, char **byt /* Returns bytes_key * *bytes_key MUST NOT BE FREED BY THE CALLER */ -esp_err_t esp_srp_get_session_key(esp_srp_handle_t *hd, char *bytes_A, int len_A, char **bytes_key, int *len_key); +esp_err_t esp_srp_get_session_key(esp_srp_handle_t *hd, char *bytes_A, int len_A, char **bytes_key, uint16_t *len_key); /* Exchange proofs * Returns 1 if user's proof is ok. Also 1 when is returned, bytes_host_proof contains our proof. diff --git a/components/protocomm/src/security/security2.c b/components/protocomm/src/security/security2.c index e13d2a7d09..aff063da65 100644 --- a/components/protocomm/src/security/security2.c +++ b/components/protocomm/src/security/security2.c @@ -90,6 +90,11 @@ static esp_err_t handle_session_command0(session_t *cur_session, return ESP_ERR_INVALID_ARG; } + if (sv == NULL) { + ESP_LOGE(TAG, "Invalid security params"); + return ESP_ERR_INVALID_ARG; + } + cur_session->username_len = in->sc0->client_username.len; cur_session->username = calloc(cur_session->username_len, sizeof(char)); if (!cur_session->username) { @@ -129,7 +134,7 @@ static esp_err_t handle_session_command0(session_t *cur_session, cur_session->verifier_len = sv->verifier_len; ESP_LOGI(TAG, "Using salt and verifier to generate public key..."); - if (sv != NULL && sv->salt != NULL && sv->salt_len != 0 && sv->verifier != NULL && sv->verifier_len != 0) { + if (sv->salt != NULL && sv->salt_len != 0 && sv->verifier != NULL && sv->verifier_len != 0) { if (esp_srp_set_salt_verifier(cur_session->srp_hd, cur_session->salt, cur_session->salt_len, cur_session->verifier, cur_session->verifier_len) != ESP_OK) { ESP_LOGE(TAG, "Failed to set salt and verifier!"); return ESP_FAIL; @@ -141,9 +146,8 @@ static esp_err_t handle_session_command0(session_t *cur_session, } hexdump("Device Public Key", device_pubkey, device_pubkey_len); - if (esp_srp_get_session_key(cur_session->srp_hd, cur_session->client_pubkey, cur_session->client_pubkey_len, - &cur_session->session_key, (int *)&cur_session->session_key_len) != ESP_OK) { + &cur_session->session_key, &cur_session->session_key_len) != ESP_OK) { ESP_LOGE(TAG, "Failed to generate device session key!"); return ESP_FAIL; } @@ -225,6 +229,7 @@ static esp_err_t handle_session_command1(session_t *cur_session, mbed_err = mbedtls_gcm_setkey(&cur_session->ctx_gcm, MBEDTLS_CIPHER_ID_AES, (unsigned char *)cur_session->session_key, AES_GCM_KEY_LEN); if (mbed_err != 0) { ESP_LOGE(TAG, "Failure at mbedtls_gcm_setkey_enc with error code : -0x%x", -mbed_err); + free(device_proof); mbedtls_gcm_free(&cur_session->ctx_gcm); return ESP_FAIL; } @@ -233,6 +238,7 @@ static esp_err_t handle_session_command1(session_t *cur_session, S2SessionResp1 *out_resp = (S2SessionResp1 *) malloc(sizeof(S2SessionResp1)); if (!out || !out_resp) { ESP_LOGE(TAG, "Error allocating memory for response1"); + free(device_proof); free(out); free(out_resp); mbedtls_gcm_free(&cur_session->ctx_gcm);