Add kernelCTF CVE-2024-57947_mitigation #162
Conversation
mingi
changed the title
koczkatamas
force-pushed
the
master
branch
3 times, most recently
from
933028d to
c111d81
Compare
koczkatamas
force-pushed
the
master
branch
13 times, most recently
from
76d6b57 to
bf0d68f
Compare
There was a problem hiding this comment.
Hey,
This is just a quick code quality review. We're planning to review the submissions more deeply (actually understanding what the exploit does) next week.
In general, the code quality looks good, no major concerns. I've left a few smaller comments here and please also apply the changes asked in the other PRs (e.g. #161) here as well.
We also have a draft style guide now. Please take a look and let us know if it's helpful for understanding our code quality expectations: https://google.github.io/security-research/kernelctf/style_guide.
Thanks for the submission and PR!
| } | ||
|
|
||
| void trigger(char * trig_set_name, char * trig_chain_name){ | ||
| uint8_t desc_concat[64] = {0,}; |
There was a problem hiding this comment.
Can you comment here in more detail what these values mean? See more here: https://google.github.io/security-research/kernelctf/style_guide#name-andor-comment-numeric-constants
|
|
||
| unsigned long user_data[0x100] = {0,}; | ||
|
|
||
| *(unsigned long*) &key_data[0] = 0xb000001010a; |
There was a problem hiding this comment.
Can you comment here in more detail what these values mean and how they are interpreted / user? Would the exploit work in the same way if we would change them or their exact value matters? See more here: https://google.github.io/security-research/kernelctf/style_guide#name-andor-comment-numeric-constants
| nftnl_set_set_u32(set, NFTNL_SET_DATA_TYPE, NFT_DATA_VERDICT); | ||
| nftnl_set_set_data(set, NFTNL_SET_DESC_CONCAT, desc_concat, 4); | ||
|
|
||
| uint8_t key_data[0x200] = {0,}; |
There was a problem hiding this comment.
What are these sizes (0x200, 0x2000, 0x100) influence? Does it matter from the cache point-of-view? Comment this here. See more here: https://google.github.io/security-research/kernelctf/style_guide#name-andor-comment-numeric-constants
| *(unsigned long*) &key_end_data[0] = 0x1400ff02010a; | ||
| *(unsigned long*) &key_end_data[8] = 17; | ||
|
|
||
| struct nftnl_set * set_elem_triggers[0x200]; |
There was a problem hiding this comment.
There seems to be an inconsistency between spraying iteration counts:
The code:
- allocates an array with
0x200==512slots for the objects - actually creates
20*20==400objects - uses
200objects in the batch
It’s not clear whether these are typos and bugs in the exploit (hex 0x200 != decimal 200) or it is intentional.
Please make the code consistent or comment here if this was intentional.
Read more here: https://google.github.io/security-research/kernelctf/style_guide#match-iteration-count-for-allocation-creation-and-usage
No description provided.