Vulnerability: Linaro/OP-TEE OP-TEE 3.3.0 and earlier is affected by: Boundary crossing. The impact is: Memory corruption of the TEE itself. The component is: optee_os. The fixed version is: 3.4.0 and later.
Vulnerability ID: CVE-2019-1010293
CWE: CWE-787
Severity score: 6.4
Project: https://github.com/OP-TEE/optee_os
Published: 2019-07-15
File: core/arch/arm/mm/tee_mmu.c
Commit message: core: tee_mmu_check_access_rights() check all pages

Prior to this patch tee_mmu_check_access_rights() checks an address in
each page of a supplied range. If both the start and length of that
range is unaligned the last page in the range is sometimes not checked.
With this patch the first address of each page in the range is checked
to simplify the logic of checking each page and the range and also to
cover the last page under all circumstances.

Fixes: OP-TEE-2018-0005: "tee_mmu_check_access_rights does not check
final page of TA buffer"

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 tee_mmu_check_access_rights(const struct user_ta_ctx *utc, 				       size_t len) { 	uaddr_t a; 	size_t addr_incr = MIN(CORE_MMU_USER_CODE_SIZE, 			       CORE_MMU_USER_PARAM_SIZE);  	if (ADD_OVERFLOW(uaddr, len, &a)) 		return TEE_ERROR_ACCESS_DENIED;  	if ((flags & TEE_MEMORY_ACCESS_NONSECURE) && TEE_Result tee_mmu_check_access_rights(const struct user_ta_ctx *utc, 	   !tee_mmu_is_vbuf_inside_ta_private(utc, (void *)uaddr, len)) 		return TEE_ERROR_ACCESS_DENIED;  	for (a = uaddr; a < (uaddr + len); a += addr_incr) { 		uint32_t attr; 		TEE_Result res; 

Fixed version (after fix):
 TEE_Result tee_mmu_check_access_rights(const struct user_ta_ctx *utc, 				       size_t len) { 	uaddr_t a; 	uaddr_t end_addr = 0; 	size_t addr_incr = MIN(CORE_MMU_USER_CODE_SIZE, 			       CORE_MMU_USER_PARAM_SIZE);  	if (ADD_OVERFLOW(uaddr, len, &end_addr)) 		return TEE_ERROR_ACCESS_DENIED;  	if ((flags & TEE_MEMORY_ACCESS_NONSECURE) && TEE_Result tee_mmu_check_access_rights(const struct user_ta_ctx *utc, 	   !tee_mmu_is_vbuf_inside_ta_private(utc, (void *)uaddr, len)) 		return TEE_ERROR_ACCESS_DENIED;  	for (a = ROUNDDOWN(uaddr, addr_incr); a < end_addr; a += addr_incr) { 		uint32_t attr; 		TEE_Result res; 

Patch:
patch: @@ -757,10 +757,11 @@ TEE_Result tee_mmu_check_access_rights(const struct user_ta_ctx *utc,
 				       size_t len)
 {
 	uaddr_t a;
+	uaddr_t end_addr = 0;
 	size_t addr_incr = MIN(CORE_MMU_USER_CODE_SIZE,
 			       CORE_MMU_USER_PARAM_SIZE);
 
-	if (ADD_OVERFLOW(uaddr, len, &a))
+	if (ADD_OVERFLOW(uaddr, len, &end_addr))
 		return TEE_ERROR_ACCESS_DENIED;
 
 	if ((flags & TEE_MEMORY_ACCESS_NONSECURE) &&
@@ -775,7 +776,7 @@ TEE_Result tee_mmu_check_access_rights(const struct user_ta_ctx *utc,
 	   !tee_mmu_is_vbuf_inside_ta_private(utc, (void *)uaddr, len))
 		return TEE_ERROR_ACCESS_DENIED;
 
-	for (a = uaddr; a < (uaddr + len); a += addr_incr) {
+	for (a = ROUNDDOWN(uaddr, addr_incr); a < end_addr; a += addr_incr) {
 		uint32_t attr;
 		TEE_Result res;
path: core/arch/arm/mm/tee_mmu.c