Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Bug fixes #158

Merged
merged 10 commits into from
Jan 17, 2019
Merged

Bug fixes #158

merged 10 commits into from
Jan 17, 2019

Conversation

hfiref0x
Copy link
Contributor

Fix multiple bugs in CreateRemoteThread_Injection
Fix multiple bugs in GetMainThreadId
Fix invalid return value check in QueueUserAPC_Injection routine
Fix invalid return value check in CreateRemoteThread_Injection routine
Update print_os routine
Fix always true expression in PrintAvailabilityReport routine
Fix resource leak in vmware_devices routine
Fix resource leak in vbox_devices routine
Fix invalid comparison in IsBadLibrary routine
Fix invalid memory allocation size in HardwareBreakpoints routine

It wasn't crashing just because VirtualAllocEx does allocation size round up it to next page boundary automatically.
filenameLength declared as DWORD - unsigned long. Expression if (filenameLength < 0) is always false and thus block was never executed.
Missing CloseHandle call added.
Missing CloseHandle call added.
if (warningCount == 0) is always true expression because warningCount assigned to 0 and never changed across function body.
The _tcscpy_s has no sense and overhead because 1st argument of this function is equal to the 3rd.
Expression if (bStatus == NULL) is always false because it was checked before.  Thus error handling block never executed. Considering wrong valuable used.
Expression if (hThread == NULL) is always true because hThread is assigned to NULL and don't change up to this line. Thus rest of the code never executed.
There is a resource leak in case of successful execution, there is an invalid handle value passed to CloseHandle in case of failure.
This is a second, more complex fix for this routine. There is a multiple resource leaks and invalid VirtualFreeEx call. Note that truncated pointers in various printfs are still in place as I don't know was it intended or not.
@ayoubfaouzi
Copy link
Owner

Thanks a lot @hfiref0x for the PR. Seems to me all good 👍

@ayoubfaouzi ayoubfaouzi merged commit 33b7005 into ayoubfaouzi:master Jan 17, 2019
@hfiref0x hfiref0x deleted the patch branch January 18, 2019 06:15
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants