CVE-2019-25052: Use of a Broken or Risky Cryptographic Algorithm (CWE-327)
value: In Linaro OP-TEE before 3.7.0, by using inconsistent or malformed data, it is possible to call update and final cryptographic functions directly, causing a crash that could leak sensitive information.
CWE description: The product uses a broken or risky cryptographic algorithm or protocol.
CVSS v3 base score: 9.1
Published: 2021-08-11T15:15Z
Repository: https://github.com/OP-TEE/optee_os
Language: C
Files changed: core/tee/tee_svc_cryp.c
Commit date: 2019-10-08 17:05:54 +0200
Commit message: cryp: prevent direct calls to update and final functions

With inconsistent or malformed data it has been possible to call
"update" and "final" crypto functions directly. Using a fuzzer tool [1]
we have seen that this results in asserts, i.e., a crash that
potentially could leak sensitive information.

By setting the state (initialized) in the crypto context (i.e., the
tee_cryp_state) at the end of all syscall_*_init functions and then add
a check of the state at the beginning of all update and final functions,
  we prevent direct entrance to the "update" and "final" functions.

[1] https://github.com/MartijnB/optee_fuzzer

Fixes: OP-TEE-2019-0021

Signed-off-by: Joakim Bech <joakim.bech@linaro.org>
Reported-by: Martijn Bogaard <bogaard@riscure.com>
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

Diff: diff --git a/core/tee/tee_svc_cryp.c b/core/tee/tee_svc_cryp.c
index cd7903667..588e20e93 100644
--- a/core/tee/tee_svc_cryp.c
+++ b/core/tee/tee_svc_cryp.c
@@ -29,19 +29,25 @@
 #endif
 #if defined(CFG_CRYPTO_PBKDF2)
 #include <tee/tee_cryp_pbkdf2.h>
 #endif
 
+enum cryp_state {
+	CRYP_STATE_INITIALIZED = 0,
+	CRYP_STATE_UNINITIALIZED
+};
+
 typedef void (*tee_cryp_ctx_finalize_func_t) (void *ctx, uint32_t algo);
 struct tee_cryp_state {
 	TAILQ_ENTRY(tee_cryp_state) link;
 	uint32_t algo;
 	uint32_t mode;
 	vaddr_t key1;
 	vaddr_t key2;
 	void *ctx;
 	tee_cryp_ctx_finalize_func_t ctx_finalize;
+	enum cryp_state state;
 };
 
 struct tee_cryp_obj_secret {
 	uint32_t key_size;
 	uint32_t alloc_size;
@@ -2054,10 +2060,11 @@ TEE_Result syscall_cryp_state_alloc(unsigned long algo, unsigned long mode,
 	if (!cs)
 		return TEE_ERROR_OUT_OF_MEMORY;
 	TAILQ_INSERT_TAIL(&utc->cryp_states, cs, link);
 	cs->algo = algo;
 	cs->mode = mode;
+	cs->state = CRYP_STATE_UNINITIALIZED;
 
 	switch (TEE_ALG_GET_CLASS(algo)) {
 	case TEE_OPERATION_EXTENSION:
 #ifdef CFG_CRYPTO_RSASSA_NA1
 		if (algo == TEE_ALG_RSASSA_PKCS1_V1_5)
@@ -2177,10 +2184,12 @@ TEE_Result syscall_cryp_state_copy(unsigned long dst, unsigned long src)
 		break;
 	default:
 		return TEE_ERROR_BAD_STATE;
 	}
 
+	cs_dst->state = cs_src->state;
+
 	return TEE_SUCCESS;
 }
 
 void tee_svc_cryp_free_states(struct user_ta_ctx *utc)
 {
@@ -2251,10 +2260,12 @@ TEE_Result syscall_hash_init(unsigned long state,
 		}
 	default:
 		return TEE_ERROR_BAD_PARAMETERS;
 	}
 
+	cs->state = CRYP_STATE_INITIALIZED;
+
 	return TEE_SUCCESS;
 }
 
 TEE_Result syscall_hash_update(unsigned long state, const void *chunk,
 			size_t chunk_size)
@@ -2284,10 +2295,13 @@ TEE_Result syscall_hash_update(unsigned long state, const void *chunk,
 
 	res = tee_svc_cryp_get_state(sess, tee_svc_uref_to_vaddr(state), &cs);
 	if (res != TEE_SUCCESS)
 		return res;
 
+	if (cs->state != CRYP_STATE_INITIALIZED)
+		return TEE_ERROR_BAD_STATE;
+
 	switch (TEE_ALG_GET_CLASS(cs->algo)) {
 	case TEE_OPERATION_DIGEST:
 		res = crypto_hash_update(cs->ctx, cs->algo, chunk, chunk_size);
 		if (res != TEE_SUCCESS)
 			return res;
@@ -2342,10 +2356,13 @@ TEE_Result syscall_hash_final(unsigned long state, const void *chunk,
 
 	res = tee_svc_cryp_get_state(sess, tee_svc_uref_to_vaddr(state), &cs);
 	if (res != TEE_SUCCESS)
 		return res;
 
+	if (cs->state != CRYP_STATE_INITIALIZED)
+		return TEE_ERROR_BAD_STATE;
+
 	switch (TEE_ALG_GET_CLASS(cs->algo)) {
 	case TEE_OPERATION_DIGEST:
 		res = tee_hash_get_digest_size(cs->algo, &hash_size);
 		if (res != TEE_SUCCESS)
 			return res;
@@ -2451,10 +2468,12 @@ TEE_Result syscall_cipher_init(unsigned long state, const void *iv,
 	}
 	if (res != TEE_SUCCESS)
 		return res;
 
 	cs->ctx_finalize = crypto_cipher_final;
+	cs->state = CRYP_STATE_INITIALIZED;
+
 	return TEE_SUCCESS;
 }
 
 static TEE_Result tee_svc_cipher_update_helper(unsigned long state,
 			bool last_block, const void *src, size_t src_len,
@@ -2471,10 +2490,13 @@ static TEE_Result tee_svc_cipher_update_helper(unsigned long state,
 
 	res = tee_svc_cryp_get_state(sess, tee_svc_uref_to_vaddr(state), &cs);
 	if (res != TEE_SUCCESS)
 		return res;
 
+	if (cs->state != CRYP_STATE_INITIALIZED)
+		return TEE_ERROR_BAD_STATE;
+
 	res = tee_mmu_check_access_rights(to_user_ta_ctx(sess->ctx),
 					  TEE_MEMORY_ACCESS_READ |
 					  TEE_MEMORY_ACCESS_ANY_OWNER,
 					  (uaddr_t)src, src_len);
 	if (res != TEE_SUCCESS)
@@ -2985,10 +3007,12 @@ TEE_Result syscall_authenc_init(unsigned long state, const void *nonce,
 				  payload_len);
 	if (res != TEE_SUCCESS)
 		return res;
 
 	cs->ctx_finalize = (tee_cryp_ctx_finalize_func_t)crypto_authenc_final;
+	cs->state = CRYP_STATE_INITIALIZED;
+
 	return TEE_SUCCESS;
 }
 
 TEE_Result syscall_authenc_update_aad(unsigned long state,
 			const void *aad_data, size_t aad_data_len)
@@ -3011,10 +3035,13 @@ TEE_Result syscall_authenc_update_aad(unsigned long state,
 
 	res = tee_svc_cryp_get_state(sess, tee_svc_uref_to_vaddr(state), &cs);
 	if (res != TEE_SUCCESS)
 		return res;
 
+	if (cs->state != CRYP_STATE_INITIALIZED)
+		return TEE_ERROR_BAD_STATE;
+
 	if (TEE_ALG_GET_CLASS(cs->algo) != TEE_OPERATION_AE)
 		return TEE_ERROR_BAD_STATE;
 
 	res = crypto_authenc_update_aad(cs->ctx, cs->algo, cs->mode,
 					aad_data, aad_data_len);
@@ -3039,10 +3066,13 @@ TEE_Result syscall_authenc_update_payload(unsigned long state,
 
 	res = tee_svc_cryp_get_state(sess, tee_svc_uref_to_vaddr(state), &cs);
 	if (res != TEE_SUCCESS)
 		return res;
 
+	if (cs->state != CRYP_STATE_INITIALIZED)
+		return TEE_ERROR_BAD_STATE;
+
 	if (TEE_ALG_GET_CLASS(cs->algo) != TEE_OPERATION_AE)
 		return TEE_ERROR_BAD_STATE;
 
 	res = tee_mmu_check_access_rights(to_user_ta_ctx(sess->ctx),
 					  TEE_MEMORY_ACCESS_READ |
@@ -3098,10 +3128,13 @@ TEE_Result syscall_authenc_enc_final(unsigned long state,
 
 	res = tee_svc_cryp_get_state(sess, tee_svc_uref_to_vaddr(state), &cs);
 	if (res != TEE_SUCCESS)
 		return res;
 
+	if (cs->state != CRYP_STATE_INITIALIZED)
+		return TEE_ERROR_BAD_STATE;
+
 	if (cs->mode != TEE_MODE_ENCRYPT)
 		return TEE_ERROR_BAD_PARAMETERS;
 
 	if (TEE_ALG_GET_CLASS(cs->algo) != TEE_OPERATION_AE)
 		return TEE_ERROR_BAD_STATE;
@@ -3182,10 +3215,13 @@ TEE_Result syscall_authenc_dec_final(unsigned long state,
 
 	res = tee_svc_cryp_get_state(sess, tee_svc_uref_to_vaddr(state), &cs);
 	if (res != TEE_SUCCESS)
 		return res;
 
+	if (cs->state != CRYP_STATE_INITIALIZED)
+		return TEE_ERROR_BAD_STATE;
+
 	if (cs->mode != TEE_MODE_DECRYPT)
 		return TEE_ERROR_BAD_PARAMETERS;
 
 	if (TEE_ALG_GET_CLASS(cs->algo) != TEE_OPERATION_AE)
 		return TEE_ERROR_BAD_STATE;

Fixed code: enum cryp_state {
	CRYP_STATE_INITIALIZED = 0,
	CRYP_STATE_UNINITIALIZED
};

	enum cryp_state state;
	cs->state = CRYP_STATE_UNINITIALIZED;
	cs_dst->state = cs_src->state;

	cs->state = CRYP_STATE_INITIALIZED;

	if (cs->state != CRYP_STATE_INITIALIZED)
		return TEE_ERROR_BAD_STATE;

	if (cs->state != CRYP_STATE_INITIALIZED)
		return TEE_ERROR_BAD_STATE;

	cs->state = CRYP_STATE_INITIALIZED;

	if (cs->state != CRYP_STATE_INITIALIZED)
		return TEE_ERROR_BAD_STATE;

	cs->state = CRYP_STATE_INITIALIZED;

	if (cs->state != CRYP_STATE_INITIALIZED)
		return TEE_ERROR_BAD_STATE;

	if (cs->state != CRYP_STATE_INITIALIZED)
		return TEE_ERROR_BAD_STATE;

	if (cs->state != CRYP_STATE_INITIALIZED)
		return TEE_ERROR_BAD_STATE;

	if (cs->state != CRYP_STATE_INITIALIZED)
		return TEE_ERROR_BAD_STATE;