CVE-2019-1010295: Improper Input Validation (CWE-20)
value: Linaro/OP-TEE OP-TEE 3.3.0 and earlier is affected by: Buffer Overflow. The impact is: Memory corruption and disclosure of memory content. The component is: optee_os. The fixed version is: 3.4.0 and later.
CWE description: The product receives input or data, but it does
        not validate or incorrectly validates that the input has the
        properties that are required to process the data safely and
        correctly.
CVSS v3 base score: 9.8
Published: 2019-07-15T18:15Z
Repository: https://github.com/OP-TEE/optee_os
Language: C
Files changed: core/tee/tee_svc.c
Commit date: 2019-01-21 18:28:37 +0100
Commit message: core: svc: always check ta parameters

Always check TA parameters from a user TA. This prevents a user TA from
passing invalid pointers to a pseudo TA.

Fixes: OP-TEE-2018-0007: "Buffer checks missing when calling pseudo
TAs".

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Tested-by: Joakim Bech <joakim.bech@linaro.org> (QEMU v7, v8)
Reviewed-by: Joakim Bech <joakim.bech@linaro.org>
Reported-by: Riscure <inforequest@riscure.com>
Reported-by: Alyssa Milburn <a.a.milburn@vu.nl>
Acked-by: Etienne Carriere <etienne.carriere@linaro.org>

Diff: diff --git a/core/tee/tee_svc.c b/core/tee/tee_svc.c
index 01392ba27..2be5b3f57 100644
--- a/core/tee/tee_svc.c
+++ b/core/tee/tee_svc.c
@@ -492,27 +492,35 @@ TEE_Result syscall_get_property_name_to_index(unsigned long prop_set,
 out:
 	free(kname);
 	return res;
 }
 
-static void utee_param_to_param(struct tee_ta_param *p, struct utee_params *up)
+static TEE_Result utee_param_to_param(struct user_ta_ctx *utc,
+				      struct tee_ta_param *p,
+				      struct utee_params *up)
 {
 	size_t n;
 	uint32_t types = up->types;
 
 	p->types = types;
 	for (n = 0; n < TEE_NUM_PARAMS; n++) {
 		uintptr_t a = up->vals[n * 2];
 		size_t b = up->vals[n * 2 + 1];
+		uint32_t flags = TEE_MEMORY_ACCESS_READ |
+				 TEE_MEMORY_ACCESS_ANY_OWNER;
 
 		switch (TEE_PARAM_TYPE_GET(types, n)) {
-		case TEE_PARAM_TYPE_MEMREF_INPUT:
 		case TEE_PARAM_TYPE_MEMREF_OUTPUT:
 		case TEE_PARAM_TYPE_MEMREF_INOUT:
+			flags |= TEE_MEMORY_ACCESS_WRITE;
+			/*FALLTHROUGH*/
+		case TEE_PARAM_TYPE_MEMREF_INPUT:
 			p->u[n].mem.mobj = &mobj_virt;
 			p->u[n].mem.offs = a;
 			p->u[n].mem.size = b;
+			if (tee_mmu_check_access_rights(utc, flags, a, b))
+				return TEE_ERROR_ACCESS_DENIED;
 			break;
 		case TEE_PARAM_TYPE_VALUE_INPUT:
 		case TEE_PARAM_TYPE_VALUE_INOUT:
 			p->u[n].val.a = a;
 			p->u[n].val.b = b;
@@ -520,10 +528,12 @@ static void utee_param_to_param(struct tee_ta_param *p, struct utee_params *up)
 		default:
 			memset(&p->u[n], 0, sizeof(p->u[n]));
 			break;
 		}
 	}
+
+	return TEE_SUCCESS;
 }
 
 static TEE_Result alloc_temp_sec_mem(size_t size, struct mobj **mobj,
 				     uint8_t **va)
 {
@@ -573,11 +583,13 @@ static TEE_Result tee_svc_copy_param(struct tee_ta_session *sess,
 		res = tee_mmu_check_access_rights(utc,
 			TEE_MEMORY_ACCESS_READ | TEE_MEMORY_ACCESS_ANY_OWNER,
 			(uaddr_t)callee_params, sizeof(struct utee_params));
 		if (res != TEE_SUCCESS)
 			return res;
-		utee_param_to_param(param, callee_params);
+		res = utee_param_to_param(utc, param, callee_params);
+		if (res != TEE_SUCCESS)
+			return res;
 	}
 
 	if (called_sess && is_pseudo_ta_ctx(called_sess->ctx)) {
 		/* pseudo TA borrows the mapping of the calling TA */
 		return TEE_SUCCESS;

Vulnerable code: static void utee_param_to_param(struct tee_ta_param *p, struct utee_params *up)
		case TEE_PARAM_TYPE_MEMREF_INPUT:
		utee_param_to_param(param, callee_params);
Fixed code: static TEE_Result utee_param_to_param(struct user_ta_ctx *utc,
				      struct tee_ta_param *p,
				      struct utee_params *up)
		uint32_t flags = TEE_MEMORY_ACCESS_READ |
				 TEE_MEMORY_ACCESS_ANY_OWNER;
			flags |= TEE_MEMORY_ACCESS_WRITE;
			/*FALLTHROUGH*/
		case TEE_PARAM_TYPE_MEMREF_INPUT:
			if (tee_mmu_check_access_rights(utc, flags, a, b))
				return TEE_ERROR_ACCESS_DENIED;

	return TEE_SUCCESS;
		res = utee_param_to_param(utc, param, callee_params);
		if (res != TEE_SUCCESS)
			return res;