Vulnerability: 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.
Vulnerability ID: CVE-2019-1010295
CWE: CWE-20
Severity score: 6.4
Project: https://github.com/OP-TEE/optee_os
Published: 2019-07-15
File: core/tee/tee_svc.c
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>
Commit author: Jens Wiklander

Vulnerable version (before fix):
 TEE_Result syscall_get_property_name_to_index(unsigned long prop_set, 	return res; }  static void utee_param_to_param(struct tee_ta_param *p, struct utee_params *up) { 	size_t n; 	uint32_t types = up->types; static void utee_param_to_param(struct tee_ta_param *p, struct utee_params *up) 	for (n = 0; n < TEE_NUM_PARAMS; n++) { 		uintptr_t a = up->vals[n * 2]; 		size_t b = up->vals[n * 2 + 1];  		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: 			p->u[n].mem.mobj = &mobj_virt; 			p->u[n].mem.offs = a; 			p->u[n].mem.size = b; 			break; 		case TEE_PARAM_TYPE_VALUE_INPUT: 		case TEE_PARAM_TYPE_VALUE_INOUT: static void utee_param_to_param(struct tee_ta_param *p, struct utee_params *up) 			break; 		} 	} }  static TEE_Result alloc_temp_sec_mem(size_t size, struct mobj **mobj, static TEE_Result tee_svc_copy_param(struct tee_ta_session *sess, 			(uaddr_t)callee_params, sizeof(struct utee_params)); 		if (res != TEE_SUCCESS) 			return res; 		utee_param_to_param(param, callee_params); 	}  	if (called_sess && is_pseudo_ta_ctx(called_sess->ctx)) {

Fixed version (after fix):
 TEE_Result syscall_get_property_name_to_index(unsigned long prop_set, 	return res; }  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; static void utee_param_to_param(struct tee_ta_param *p, struct utee_params *up) 	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_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: static void utee_param_to_param(struct tee_ta_param *p, struct utee_params *up) 			break; 		} 	}  	return TEE_SUCCESS; }  static TEE_Result alloc_temp_sec_mem(size_t size, struct mobj **mobj, static TEE_Result tee_svc_copy_param(struct tee_ta_session *sess, 			(uaddr_t)callee_params, sizeof(struct utee_params)); 		if (res != TEE_SUCCESS) 			return res; 		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)) {

Patch:
patch: @@ -494,7 +494,9 @@ TEE_Result syscall_get_property_name_to_index(unsigned long prop_set,
 	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;
@@ -503,14 +505,20 @@ static void utee_param_to_param(struct tee_ta_param *p, struct utee_params *up)
 	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:
@@ -522,6 +530,8 @@ static void utee_param_to_param(struct tee_ta_param *p, struct utee_params *up)
 			break;
 		}
 	}
+
+	return TEE_SUCCESS;
 }
 
 static TEE_Result alloc_temp_sec_mem(size_t size, struct mobj **mobj,
@@ -575,7 +585,9 @@ static TEE_Result tee_svc_copy_param(struct tee_ta_session *sess,
 			(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)) {
path: core/tee/tee_svc.c