Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flaw in vMPU access check can cause HardFault when processing a malicious IPC request #484

Open
fangyi-zhou opened this issue Aug 11, 2017 · 1 comment
Labels

Comments

@fangyi-zhou
Copy link

Architecture affected: Armv7m
Target tested: EFM32

Code used to trigger the fault:

    uvisor_ipc_desc_t send;
    send.box_id = uvisor_box_id_self();
    send.port = 1234;
    send.len = sizeof(int);
    int num_to_send = 0xdeadbeef;
    int send_ret = ipc_send(&send, &num_to_send);

    uvisor_ipc_desc_t recv;
    recv.box_id = UVISOR_BOX_ID_ANY;
    recv.port = 1234;
    recv.len = sizeof(int);
    int recv_ret = ipc_recv(&recv, 0);

Sequence to error:
(0) Flash memory initialised to be non-writable at vMPU initialisation.

UVISOR_TACLDEF_SECURE_CONST | UVISOR_TACL_EXECUTE,

(1) Send/recv requests get queued up in the correspondent IPC queue
(2) uVisor drains IPC queue to find two matching requests
(3) uVisor validates receiving IPC IO request at
if (!ipc_io_is_ok(recv_box_id, recv_io)) {

(4) uVisor validates whether the data pointer is allowed for access for given length at
vmpu_buffer_access_is_ok(box_id, io->msg, io->desc->len);

(5) vMPU checks whether public box can access the memory at
if (vmpu_buffer_access_is_ok(0, addr, size)) {

(6) vMPU checks whether the memory is in static region at
if (vmpu_buffer_access_is_ok_static(start_addr, end_addr)) {

(7) Check passes, because 0 is within the range of flash (static region 0) and end address is also in range.
(8) uVisor delivers message, triggers memory access violation due to memcpy
memcpy(recv_io->msg, send_io->msg, len);

Debug prints:

uVisor mode: 2
uvisor_ram : @0x20000000 (8192 bytes) [config]
             @0x20000000 (8192 bytes) [linker]
bss_boxes  : @0x20002000 (57344 bytes) [config]

box stack segment start=0x20002000 end=0x20010000 (length=57344)
MPU.REGIONS=8
MPU.ALIGNMENT=0x0000001F
MPU.ALIGNMENT_BITS=5
* MPU CONFIGURATION

  Background region enabled
  MPU bypassed @NMI, @HardFault
  MPU enabled

  Region Start      Size  XN AP  TEX S C B SRD      Valid
  0      0x00000000 256KB 0  110 000 0 0 0 00000000 1
  1      0x20000000 128KB 0  011 000 0 0 0 00011111 1
  2      0x00000000 000B  0  000 000 0 0 0 00000000 0
  3      0x00000000 000B  0  000 000 0 0 0 00000000 0
  4      0x00000000 000B  0  000 000 0 0 0 00000000 0
  5      0x00000000 000B  0  000 000 0 0 0 00000000 0
  6      0x00000000 000B  0  000 000 0 0 0 00000000 0
  7      0x00000000 000B  0  000 000 0 0 0 00000000 0

Box 0 ACLs:
  - Peripheral: 0x400C8000 - 0x400C8088 (permissions: 0x0AB6)
  - Peripheral: 0x400C0000 - 0x400C0058 (permissions: 0x0AB6)
  - Peripheral: 0x40006000 - 0x40006140 (permissions: 0x0AB6)
  - Peripheral: 0x40010000 - 0x4001008C (permissions: 0x0AB6)
  - Peripheral: 0x4000E000 - 0x4000E060 (permissions: 0x0AB6)
  - Peripheral: 0x0FE08000 - 0x0FE09000 (permissions: 0x0AB6)
  - Peripheral: 0x42000000 - 0x44000000 (permissions: 0x0AB6)
Box 1 ACLs:
  - SRAM:       0x20004000 - 0x20008000 (permissions: 0x0076, subregions: 0x10)
    - BSS:      0x20004000 - 0x20005D3C (original size: 7484B, rounded size: 8192B)
    - Stack:    0x20006800 - 0x20008000 (original size: 1024B, rounded size: 6144B)
Box 2 ACLs:
  - SRAM:       0x20008000 - 0x2000C000 (permissions: 0x0076, subregions: 0x10)
    - BSS:      0x20008000 - 0x2000993C (original size: 6460B, rounded size: 8192B)
    - Stack:    0x2000A800 - 0x2000C000 (original size: 1024B, rounded size: 6144B)
Box 3 ACLs:
  - SRAM:       0x2000C000 - 0x20010000 (permissions: 0x0076, subregions: 0x10)
    - BSS:      0x2000C000 - 0x2000D948 (original size: 6472B, rounded size: 8192B)
    - Stack:    0x2000E800 - 0x20010000 (original size: 1024B, rounded size: 6144B)
vmpu_enumerate_boxes [DONE]
page heap: [0x20010000, 0x20014000] 16kB -> 16 1kB pages
uvisor initialized
IRQ 20 registered to box 0
IRQ 21 registered to box 0
IRQ 21 priority set to 3 (NVIC), 1 (virtual)

***********************************************************
                    HARD FAULT
***********************************************************

* Active Box ID: 1
* FAULT SYNDROME REGISTERS

  HFSR: 0x40000000
  --> FORCED: another priority escalated to hard fault.
  --> Escalated from SysTick IRQ.

* EXCEPTION STACK FRAME

  lr: 0xFFFFFFF1
  --> FP stack frame not saved.
  --> Exception from P mode.
  --> Return to MSP.

  sp: 0x20001EF0
      sp[07]: 0x6100000B | xPSR
      sp[06]: 0x00007E0C | pc
      sp[05]: 0x00001773 | lr
      sp[04]: 0x00000000 | r12
      sp[03]: 0xDEADBEEF | r3
      sp[02]: 0x00000000 | r2
      sp[01]: 0x20005014 | r1
      sp[00]: 0x00000000 | r0


* MPU CONFIGURATION

  Background region enabled
  MPU bypassed @NMI, @HardFault
  MPU enabled

  Region Start      Size  XN AP  TEX S C B SRD      Valid
  0      0x00000000 256KB 0  110 000 0 0 0 00000000 1
  1      0x20000000 128KB 0  011 000 0 0 0 00011111 1
  2      0x00000000 000B  0  000 000 0 0 0 00000000 0
  3      0x20004000 016KB 1  011 000 0 0 0 00010000 1
  4      0x00000000 000B  0  000 000 0 0 0 00000000 0
  5      0x00000000 000B  0  000 000 0 0 0 00000000 0
  6      0x00000000 000B  0  000 000 0 0 0 00000000 0
  7      0x00000000 000B  0  000 000 0 0 0 00000000 0

***********************************************************

HALT_ERROR(./core/vmpu/src/mpu_armv7m/vmpu_armv7m.c#141): Cannot recover from a hard fault.

Suggested Fix:
If the flash is public (as in Armv8m), fix the ACL for public flash memory
Otherwise, fix access check to take parameter indicating the permission needed and check whether the operation is permitted against the ACL.

@ciarmcom
Copy link
Member

ARM Internal Ref: IOTSEC-416

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants