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

Confused-deputy attack on sm_isr.s #38

Open
jovanbulck opened this issue Oct 8, 2021 · 3 comments
Open

Confused-deputy attack on sm_isr.s #38

jovanbulck opened this issue Oct 8, 2021 · 3 comments
Labels

Comments

@jovanbulck
Copy link
Member

Should not use untrusted stack to do reti to interrrupted context. should probably move that to a untrusted_isr_reti stub outside the enclave or so:

 47     ; Switch the stack.
 48     mov r1, &__sm_sp
 49     mov &__sm_irq_sp, r1
 50     reti
@jovanbulck jovanbulck added the bug label Oct 8, 2021
@fritzalder
Copy link
Member

Also, the &__sm_sp should not be blindly overwritten as it is used as a marker for OCALLs now.

@fritzalder
Copy link
Member

I opened #39 to address the __sm_sp issue.

I also did not address the above issue in the PR yet. In my opinion, the return address on the stack can be trusted as it was placed by hardware. So one solution can also be to store the return address somewhere inside the enclave before branching to the ISR handler (that may perform an OCALL leading to the value not being trustworthy anymore).

@jovanbulck
Copy link
Member Author

I opened #39 to address the __sm_sp issue.

merged, thanks!

I also did not address the above issue in the PR yet. In my opinion, the return address on the stack can be trusted as it was placed by hardware. So one solution can also be to store the return address somewhere inside the enclave before branching to the ISR handler (that may perform an OCALL leading to the value not being trustworthy anymore).

yes but this is a slippery slope, the value that's pushed to untrusted memory, where it may be modified by OCALLs or interrupts as you said, but also by eg untrusted DMA accesses.

So the value should be inherently untrusted. We can of course still load it inside the enclave and then properly check it lies outside, as we do with the normal ecall return address, but then we can also not use reti cause there might be a time-of-check-time-of-use

All by all the cleanest solution IMO still is to do the reti in an untrusted trampoline stub at a fixed address outside the enclave

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

No branches or pull requests

2 participants