Skip to content

fix: Validate TA-supplied pointers in secure-kernel syscalls - #252

Merged
tdrozdovsky merged 2 commits into
Samsung:masterfrom
bmolodan:security/ta-pointer-access-rights
Aug 5, 2026
Merged

fix: Validate TA-supplied pointers in secure-kernel syscalls#252
tdrozdovsky merged 2 commits into
Samsung:masterfrom
bmolodan:security/ta-pointer-access-rights

Conversation

@bmolodan

@bmolodan bmolodan commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

The copy helpers tee_svc_copy_to_user/tee_svc_copy_from_user and the live crypto syscalls (utee_hash_update/final, utee_cipher_init, cipher update, copy_in_attrs) dereferenced or memcpy'd TA-supplied pointer ranges with their tee_mmu_check_access_rights() guard commented out. A malicious or buggy TA could pass a NULL or wrapping range and drive an out-of-bounds copy in the secure world.

This port has no per-TA MMU/MPU context or region table, and every call site requests TEE_MEMORY_ACCESS_ANY_OWNER, so ownership cannot (and is not meant to) be enforced. Implement tee_mmu_check_access_rights() as the accessibility check that is meaningful here - reject NULL+len and address-space-wrapping ranges, mirroring cmse_check_address_range()'s end-of-range test on the non-secure boundary - and restore the guard at every live call site, including the copy_in_attrs path that stores unvalidated attribute buffers later memcpy'd by op_attr_secret_value_from_user.

Add a temporary CONFIG_APPS_ACCESS_RIGHTS_TEST negative test (mps2 AN505) that drives the guard with malformed ranges and asserts TEE_ERROR_ACCESS_DENIED.

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code cleanup/refactoring
  • CI system update
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A
  • Test B

Test Configuration:

  • Firmware version:
  • Hardware:
  • Toolchain:
  • SDK:

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

The copy helpers tee_svc_copy_to_user/tee_svc_copy_from_user and the live
crypto syscalls (utee_hash_update/final, utee_cipher_init, cipher update,
copy_in_attrs) dereferenced or memcpy'd TA-supplied pointer ranges with their
tee_mmu_check_access_rights() guard commented out. A malicious or buggy TA
could pass a NULL or wrapping range and drive an out-of-bounds copy in the
secure world.

This port has no per-TA MMU/MPU context or region table, and every call site
requests TEE_MEMORY_ACCESS_ANY_OWNER, so ownership cannot (and is not meant to)
be enforced. Implement tee_mmu_check_access_rights() as the accessibility check
that is meaningful here - reject NULL+len and address-space-wrapping ranges,
mirroring cmse_check_address_range()'s end-of-range test on the non-secure
boundary - and restore the guard at every live call site, including the
copy_in_attrs path that stores unvalidated attribute buffers later memcpy'd by
op_attr_secret_value_from_user.

Add a temporary CONFIG_APPS_ACCESS_RIGHTS_TEST negative test (mps2 AN505) that
drives the guard with malformed ranges and asserts TEE_ERROR_ACCESS_DENIED.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@bmolodan
bmolodan requested a review from tdrozdovsky as a code owner August 3, 2026 14:21
The CONFIG_APPS_ACCESS_RIGHTS_TEST app was a throwaway negative test for the
guard added in the previous commit. The guard is now verified (host logic
tests + on-target mps2 AN505 QEMU run, all cases rejected/accepted as
expected), so drop the app and its build/menu wiring. The core guard in
tee_svc.c / tee_svc_cryp.c is unaffected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@bmolodan bmolodan changed the title tee: validate TA-supplied pointers in secure-kernel syscalls fix: Validate TA-supplied pointers in secure-kernel syscalls Aug 5, 2026
@tdrozdovsky
tdrozdovsky merged commit 102d3dc into Samsung:master Aug 5, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants