search

shakevsky / keybuster

Apache License 2.0
113 stars 31 forks source link

Adjust range of tag check in should_mark_hidden_tag #3

Closed celine-lee closed 2 years ago

celine-lee commented 2 years ago

Our team is working with your code and we noticed this if logical expression:

`if (0x90001451 == tag || (0x900002c6 <= tag && tag < 0x900002ce) || (tag < 0x9000138f && 0x900003e8 >= tag))`

In the second predicate, (tag < 0x9000138f && 0x900003e8 >= tag), the second condition seems to be included by the first, since 0x900003e8 is a smaller value than 0x9000138f. This predicate seems to translate to: 

`(tag < 0x9000138f && tag <= 0x900003e8)`

which is semantically equivalent to: tag < 0x9000138f because tag <= 0x900003e8 is subsumed by the first comparison.

We speculate that given the context of the function, this line may instead be intended as:

if (0x90001451 == tag || (0x900002c6 <= tag && tag < 0x900002ce) || (0x900003e8 <= tag && tag < 0x9000138f))

Do you agree / disagree?

Irrespective of the answer, it seems the original form has an unnecessary component / expression.

Thank you!

shakevsky commented 2 years ago

Thank you for your interest in our research! The should_mark_hidden_tag function was not relevant to Keybuster as a tool to interact with the Keymaster TA (and to reproduce our attacks).

The code of km_mark_hidden_tags and should_mark_hidden_tag was left for completeness - mostly to show that KM_TAG_EKEY_BLOB_* tags (and other "sensitive" fields) are omitted from blobs that the Keymaster TA returns to the Normal World.

You're probably right that this weird condition is incorrect (the correct condition can be found by reverse-engineering of the Keymaster TA ELF), however this code is not used so the better solution would be to comment it out in a later release. Thus, I'm closing this PR for now.

Let me know if you have a possible use-case for those functions or other thoughts on the subject. I would also appreciate to hear how your team works on the code (for what purpose/general context) - feel free to contact me via email or other mediums.