Closed kkent030315 closed 3 years ago
Excellent work. Also, thank you for the clean write up and testing on multiple platforms. Left few minor style related requests and a request to update of README.md. Otherwise, looks great.
EDIT: I also do not care Windows 7 and 8, as well as supporting a different version of capcom.sys that might have those functions at different offsets if such versions exist. I tested with the version noted in README.md and that's all this project needs to support.
I forgot to implement mutex for PsLoadedModuleList
the code must acquire PsLoadedModuleResource
in order to reference it.
I'll commit some additional implementation for it and README.md later.
Retested on all listed platforms and this PR is ready to go. Please review changes. Thank you.
Thanks for the update. Please address review comments
I don't see any review comments on the commits regarding your style-related requests.
Where especially should I change?
Thank you.
Apologies, I thought I submitted my comments (because you addressed some of them :) ) but it was not the case. Those comments should be visible to you now.
Code refactored.
Here is the changes:
KeEnterCriticalRegion
, KeLeaveCriticalRegion
, ExAcquireResourceExclusiveLite
, ExReleaseResourceList
ExEnterCriticalRegionAndAcquireResourceExclusive
, ExReleaseResourceAndLeaveCriticalRegion
for simpler codebasememcpy
instead of NtosRtlCopyMemory
CR0.WP
disabling in order to safer execution of patching .text sectionPlease re-review changes. Thank you for taking time for this PR. :)
This is a bit unrelated but I'd like to share information here.
From my investigation, it looks like the whole image that mapped to the KVA is writable. (Even IAT or PE, whatever) On other normal driver, it is no way this thing happens.
During my reverse engineering of this driver I thought that the Capcom image size is very small (0xE00
). something is special with this driver.
EDIT 19th Aug 2021:
I assume capcom.sys
intentionally changes /ALIGN
linker option to make binary minimum. which will result in the entire driver image is on the single page. It makes sense now.
Such linker option would make HLK to be failure for specific versions of Windows 10.
That's interesting. Thanks for additional notes. I noticed the entire driver image is within in 0x1000 on memory and some of sections are writable. That would make the entire page, including the .text section, writable.
Love into Capcom.sys. It was vulnerable by design, had an UAF bug and the whole image was writable. Amazing quality of engineering.
Given that, I prefer not to change WP. I am pretty sure that fact that the driver is writable would be consistent across platforms, so no value to add complexity of bit flipping.
I agreed with that and removed no-need CR0.WP
modification.
Let me know if it there are another change requests. :)
Merged the PR. Thank you for analyzing the issue and working for the fix with me!
I am glad to find out I can still use this garbage as an example of.. garbage :-)
This is a mitigation for issue #3 .
Added optional augment of
--mitigatedv
which allows execution of this exploit to be work while DV (Driver Verifier) is enabled. This mitigation only supports Windows 10+.Tested on:
What
As discussed on the issue, this is due to bad implementation of capcom.
Both of
CpCreateClose
andCpDeviceIoControl
referenceIrp->IoStatus.Status
after the IRP is freed byIofCompleteRequest
.This causes page fault bugcheck on DV enabled environment. (bugcheck 0x50)
Mitigation
As the author says, we could directly patch the mapped image to avoid page faults. We need to patch two functions,
CpCreateClose
andCpDeviceIoControl
.Simply patch with following shellcode:
Problem
This mitigation will not work on either Windows 7 nor Windows 8, 8.1. I do not see any reason to compatible with these versions so I don't care.
If you have any problems or better idea, feel like to discuss here. Thank you.