Add kernelCTF CVE-2024-50264_lts_cos #145
Conversation
koczkatamas
force-pushed
the
master
branch
3 times, most recently
from
933028d to
c111d81
Compare
koczkatamas
force-pushed
the
master
branch
4 times, most recently
from
a70683f to
fe221aa
Compare
koczkatamas
force-pushed
the
master
branch
9 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 in its current form does not match the requirements to be reviewed or accepted. Please follow our (draft) style guide to fix your code: https://google.github.io/security-research/kernelctf/style_guide. Also make sure you are meeting the documentation requirements of the rules.
I've left a few explicit comments too, but it's not enough to only fix those.
Thanks for the submission and PR!
| @@ -0,0 +1,851 @@ | |||
| #define _GNU_SOURCE | |||
| #include <stdio.h> | |||
There was a problem hiding this comment.
Most of the files are included twice, some of the three times. Please remove the unnecessary includes.
| void *heap_spray_stack = NULL; | ||
| volatile int status_spray = SPRAY_ERROR; | ||
|
|
||
| int timefds[0x500000]; |
There was a problem hiding this comment.
Only allocate as many array items as you need, this uses ~20MB of RAM most of which are unnecessary.
See the related style guide entry.
| void *heap_spray_stack = NULL; | ||
| volatile int status_spray = SPRAY_ERROR; | ||
|
|
||
| int timefds[0x500000]; |
There was a problem hiding this comment.
The following variables could've been local variables: timefds, epfds, buf, targetstr, move them into local functions. Read more here: https://google.github.io/security-research/kernelctf/style_guide#usage-of-global-variables-instead-of-local-ones
| } | ||
| struct sock_filter filter[0x1000]; | ||
| char buf[0x1000]; | ||
| #define SYSCHK(x) \ |
There was a problem hiding this comment.
Put defines like this at the top of the file.
|
|
||
| int timefds[0x500000]; | ||
| int epfds[0x500000]; | ||
| char buf[0x1000]; |
There was a problem hiding this comment.
This global variable is too generic, its name conflicts & shadowed with local variables. Consider renaming it to a descriptive name which tells its purpose (e.g. thread_sync_buf).
Relevant style guide entries:
| printf("[%lld.%lld] victim_slab_to_buddy end\n", tp.tv_sec, tp.tv_nsec); | ||
| } | ||
|
|
||
| #define SEQ_FILE_CNT 0x300 |
There was a problem hiding this comment.
This code seems to be not used, please remove it: https://google.github.io/security-research/kernelctf/style_guide#unused-code
| } | ||
|
|
||
| printf("inuse: %d\n", inuse); | ||
| usleep(10000); |
There was a problem hiding this comment.
Please comment here what you are waiting for (which kernel function): https://google.github.io/security-research/kernelctf/style_guide#sleeping--waiting
| } | ||
|
|
||
| while(true) | ||
| sleep(100); |
There was a problem hiding this comment.
Please comment here what you are waiting for, would the kernel crash if you would not wait?
| int main(int argc, void *argv[]) | ||
| { | ||
|
|
||
| alarm(120); |
There was a problem hiding this comment.
Comment here what you are using the alarm for.
| if((socks[i] = socket(AF_VSOCK, SOCK_SEQPACKET, 0)) < 0) | ||
| perror("socket"); | ||
|
|
||
| unsigned long size = 0xeeff0011; |
There was a problem hiding this comment.
Comment here why you are doing this and how did you choose this specific size.
|
Also the COS exploit failed to repro. Any chance of being able to improve stability? |
No description provided.