Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

PatchImport, IMAGE_IMPORT_DESCRIPTOR end of array determined by wrong structure member #1

Open
wants to merge 326 commits into
base: master
Choose a base branch
from

Conversation

fastman92
Copy link

AVERM.dll problem
AVERM.dll loads, when loading a movie in GTA games.

It was found during the testing, that Visual Leak Detector for Win32 will crash the game, when the following file loads:
C:\Windows\SysWOW64\AVERM.dll
This file is COM registered library, one of DirectShow codecs.
"RealAudio Decoder",{941A4793-A705-4312-8DFC-C11CA05F397E},00400000,"C:\Windows\SysWow64\AVERM.dll"
"RealMedia Source",{765035B3-5944-4A94-806B-20EE3415F26F},00600000,"C:\Windows\SysWow64\AVERM.dll"
"RealMedia Splitter",{E21BE468-5C18-43EB-B0CC-DB93A847D769},00600000,"C:\Windows\SysWow64\AVERM.dll"
"RealVideo Decoder",{238D0F23-5DC9-45A6-9BE2-666160C324DD},00400000,"C:\Windows\SysWow64\AVERM.dll"

AVERM.dll is packed by UPX compressor.
The VLD plugin crashed within PatchImport function with HMODULE address of AVERM.dll.
Initially I have expected that one of my codec files is crashing my application.
After a debugging process, which took me 2 days I have figured out what’s wrong.

Solution:
File vld-master\src
Function:
BOOL PatchImport (HMODULE importmodule, moduleentry_t *patchModule)
Find:
while (idte->FirstThunk != 0x0) {
Change into:
while (idte->OriginalFirstThunk != 0x0) {

Explanation:
An array of _IMAGE_IMPORT_DESCRIPTOR finishes, when the first structure member is filled with zeros:

    union {
        DWORD   Characteristics;            // 0 for terminating null import descriptor
        DWORD   OriginalFirstThunk;         // RVA to original unbound IAT (PIMAGE_THUNK_DATA)
    } DUMMYUNIONNAME;

Not this one, which has got a similar name:
`DWORD FirstThunk; // RVA to IAT (if bound this IAT has actual addresses)

`
Now AVERM.dll is the library, where the last entry has got FirstThunk with some random value and proper OriginalFirstThunk=0. The Visual Leak Detector was reading an invalid value and executing FindRealCode badly.

Chris Johnson and others added 30 commits October 10, 2012 03:54
…c application to the test folder in visual studio.

Ran all the tests before submitting.
testsuite.exe is still broken, and still throwing an assert during execution.

git-tfs-id: [https://tfs.codeplex.com/tfs/TFS07]$/vld/vld/trunk;C66359
One especially problematic way to crash an application is to allocate memory in one heap, and free it in another.
The MS debug heaps do detect this, but they do NOT report any callstacks indicating the allocation callstack. The MS debug heaps simply report an error condition, and issue a break leaving the end user guessing what went wrong. 

Visual leak detector can now pinpoint EXACTLY report on such a situation by reporting callstacks of the allocation and deallocation of the memory that corrupted the heap.

If a memory allocation is unmapped with the wrong heap handle, VLD will do a search through it's internal maps of memory allocations searching for the memory address. It will also find the heap handle that IS associated with the memory block.
This of course comes at a cost, as there are many heap allocs and heap frees that VLD does not monitor. To seperate the two, and keep performance similar to what it was before, this extra checking must be explicitely be turned on via INI file, or by setting:
VLD_OPT_VALIDATE_HEAPFREE
via VLDSetOptions.

I also wrote a unit test to verify this extra behavior. This test is called corruptions, and will be the test bed for further testing in VLD.

I also cleaned up the silly semantics of the strapp method (what a stupid name), renaming it StringAppend, and having it return a string, instead of passing in a string by reference. Which goes to prove that trying to be really clever just makes everyone confused.

TESTS:
Ran all tests in 32 bit, and all passed.

git-tfs-id: [https://tfs.codeplex.com/tfs/TFS07]$/vld/vld/trunk;C66462
…ion feature. This resulted in printing an extra stack frame at the top of the stack that was internal to VLD, and not part of the users code.

Please run all unit tests before checking in, otherwise quality will degrade.

git-tfs-id: [https://tfs.codeplex.com/tfs/TFS07]$/vld/vld/trunk;C66676
…hreads.

I have already logged an issue (8636).
To reproduce the crash, run dynamic_app with "true" and "thread" for the parameters.

git-tfs-id: [https://tfs.codeplex.com/tfs/TFS07]$/vld/vld/trunk;C66942
…blems.

I don't know why this wasn't turned on before, but this is a very powerful static analyzer that finds subtle problems.
I'm turning this on for 32 bit debug builds.

ThreadTest.cpp - 
Fixed some comments

dynamic_app.cpp - 
Change the number of leaks that should be found.

vld.cpp - 
Added guards against possible NULL module handles passed to GetProcAddress.
Added a structure exception filter function for use in an __except clause.
Also added a guard against dereferencing a NULL crtdbgblockheader_t pointer.
Other inconsequential changes to comments and stuff.

utility.h - 
Added a declaration for a filter function to be used in structured exception handling

utility.cpp
Used filter function (that does nothing but continue the search for an exception handler) for __except statements, instead of constant epressions.
Added a guard to protect against using a NULL module handler in a callt o GetProcAddress.

criticalsection.h - 
Added a leave method to the CriticalSectionLocker class, so that it can call leave before the end of the scope. This class is written to ensure exception safety, but the additional method lets us limit the scope of the critical section usage. So you get the best of both features.

callstack.h - 
removed an archaic typedef struct

callstack.cpp
We were passing in an incorrect buffer size parameter to GetModuleFileName. Thanks to code analysis for catching this one.
Added a better guard against overrunning the chunck frames array.

git-tfs-id: [https://tfs.codeplex.com/tfs/TFS07]$/vld/vld/trunk;C67192
ResolveCallstacks fixed with aggregate duplicates

git-tfs-id: [https://tfs.codeplex.com/tfs/TFS07]$/vld/vld/trunk;C67200
… VLD even if not running in DEBUG

* Added support release CRT runtime
* Added new API function called VLDReportAlloc
* Added new API function called VLDReportFree
* m_maplock renamed to m_heapmaplock

I wanted to include updated project files, but my Visual Studio Express doesn't support 64 bit compiler, and ruins the project files.

Requirements for detecting memory leaks in Release-builds:
- "Omit Frame Pointers" optimization must be turned off for the testing application (Also whole program optimization)
- Debug information must be turned on in the compiler- AND linker-options
- Define VLD_FORCE_ENABLE before #include <vld.h>
Patch by snakefoot plus my few improvements/changes

git-tfs-id: [https://tfs.codeplex.com/tfs/TFS07]$/vld/vld/trunk;C67239
…mmits (reduced thread locking time)

ReportAlloc/ReportFree functions removed

git-tfs-id: [https://tfs.codeplex.com/tfs/TFS07]$/vld/vld/trunk;C67547
Undecorating symbols changed
VLDGetLeaksCount now API function
Memory leaks additional statistic added

git-tfs-id: [https://tfs.codeplex.com/tfs/TFS07]$/vld/vld/trunk;C67800
dmitriy.vovk and others added 30 commits November 25, 2015 14:16
Fixed crash, when VLD is turned off through config
Removed src from installer
…ng crashes because EntryPoint is called through __guard_dispatch_icall_fptr()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants