CVE-2019-1010295: Out-of-bounds Read (CWE-125)
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 reads data past the end, or before the beginning, of the intended buffer.
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;