Conversation
…arget SDK version
… be sufficient to trigger the race condition
Fuzion24
left a comment
Fuzion24
left a comment
There was a problem hiding this comment.
Hey there. Thank you for your contribution. I added some comments pointing out some bugs. If you'd go ahead and fix those, I'd gladly accept the pull request.
Cheers,
Ryan
| } | ||
| if (fstat(oldFile,&st_oldFile) == -1) { | ||
| LOGV("could not open %s", argv[0]); | ||
| return cantOpenFile; |
There was a problem hiding this comment.
You leave oldfile open here.
| } | ||
| if (fstat(newFile,&st_newFile) == -1) { | ||
| LOGV("could not open %s", argv[1]); | ||
| return cantOpenFile; |
| int newFile=open(argv[1],O_RDONLY); | ||
| if (newFile == -1) { | ||
| LOGV("could not open %s", argv[1]); | ||
| return cantOpenFile; |
|
|
||
| LOGV("size %d\n\n",size); | ||
|
|
||
| mem_arg.patch = malloc(size); |
There was a problem hiding this comment.
This memory is never freed
app/src/main/jni/dirtycow.c
Outdated
|
|
||
| memset(mem_arg.patch, 0, size); | ||
|
|
||
| mem_arg.unpatch = malloc(size); |
| void * map = mmap(NULL, size, PROT_READ, MAP_PRIVATE, oldFile, 0); | ||
| if (map == MAP_FAILED) { | ||
| LOGV("mmap"); | ||
| return cantMap; |
There was a problem hiding this comment.
also, there is no corresponding munmap
app/src/main/jni/dirtycow.c
Outdated
| LOGV("open(\"/proc/self/mem\""); | ||
| for (i = 0; i < LOOP; i++) { | ||
| lseek(fd, (off_t) mem_arg->offset, SEEK_SET); | ||
| c += write(fd, p, mem_arg->patch_size); |
There was a problem hiding this comment.
This thread loops and writes no matter whether the private mapping was written to or not. You should check after each write if the r/o mapping was written to and bail on this loop if that is the case.
app/src/main/jni/dirtycow.c
Outdated
| pthread_create(&pth1, NULL, madviseThread, mem_arg); | ||
| pthread_create(&pth2, NULL, procselfmemThread, mem_arg); | ||
|
|
||
| pthread_join(pth1, NULL); |
There was a problem hiding this comment.
If the procselfthread completes, you should kill the madvise thread and bail. success or no.
| char dest[STRINGSIZE]; | ||
| const char *argv[2]; | ||
| jstring string[2]; | ||
| argv[0] = dest; |
There was a problem hiding this comment.
Rather than using two files and trying to copy one over the other. Why dont you just mmap one r/o file, and test writing to it with a static buffer of 0x41 or something.
There was a problem hiding this comment.
(just a suggestion) You could have java land open the file descriptor to the target readonly file and just pass that in rather than the file name.
Add thread to check periodically whether or not the exploit was successful Adjust maximal attempt count
|
I fixed the bugs you pointed out. Thanks, those were horrible... |
|
@iRave: Any update? |
|
@ale5000-git as I pointed out in my comment, I finished the implementation and fixed the issues @Fuzion24 pointed out. I tested the code with different devices and kernels and it works reliably. From my side, this should be good to merge. |
Fix dirtyCow filename mismatch (dirtyCow.c=>dirtycow.c)
| File file; | ||
| file = new File(context.getFilesDir(), file1Name); | ||
| file.delete(); | ||
| file = new File(context.getFilesDir(), file1Name); |
There was a problem hiding this comment.
This should be file2Name I think (yeah, I know that this repo is pretty dead, but still 😉 )
5 participants
Add Dirty COW vulnerability test to the VTS App
I used CMake to build the C code for the test, however, I also added the the necessary entry inside the Android.mk you normally use to build your native libraries. I used CMake to make the code debuggable and because it's the recommended way of building native code in android-studio. Please feel free to remove this though.