[PATCH v4 1/4] wdscore: Implement CurrentIP.

[Gabriel Ivăncescu]

On 29/01/2022 15:02, Saulius Krasuckas wrote:
> * On 2022-01-29 06:35, Mohamad Al-Jaf wrote:
> 
>> Yes, the patch solves the bug. Well, for the tests removing WINAPI 
>> seems to
>> change the pointer a bit, but it's still within the range so it doesn't
>> seem to affect it.
>    ...
>> I don't know what to do now. If it's okay to use WINAPI, should I 
>> resubmit
>> the older version of this patch?
> 
> My 2c€ worth idea is to adhere with the K.I.S.S. principle:
> just keep the current version and let the function naturally be tested 
> as much as possible.
> 
> In case there occurs some bug eg. one containing stack overflow in the 
> app (or something else) related to possibly incorrect use of 
> CurrentIP(), this would be good stimuli for enhancing the implementation 
> (and the tests).  I continue below.
> 
> * On 2022-01-21 01:03, you also asked Dmitry:
> 
>>> the test doesn't verify that returned
>> value belongs to the dll address range
>>
>> Could you explain this?
> 
> As Dmitry seemingly haven't responded to the list, I remind about this 
> by sharing my understanding.
> 
> Every process has its own virtual address space (VAS) [1].
> 
> The .exe file and each linked dll is mapped into a VAS and is given its 
> own address (so-called ImageBase) and its size here (during the load 
> time). Image base and size can be seen in the output of programs like 
> ListDlls [2], CurrProcess [3].
> 
> The same pair or numbers can be calculated for every section of a dll 
> (green boxes in the "Memory Map" pic [4]) or at least for executable 
> sections only (red boxes in Figure 2 in [5]).
> 
> It also can be done for every function exported by a dll [6].
> 
> Dmitry probably wanted to see whether the returned address belong:
> 
> - to any of "Dynamic-base DLLs"; [1]
> - to the .text section of them;
> - to the the whole .exe file;
> - to the .text section of it;
> - to specifically the wdscore.dll;
> - to the .text section of it;
> - to the specific function of specific image;
> - to the stack of some (main) thread of the process; [1]
> 
> This would require enumerating dlls mapped into the test process and 
> extracting their image bases and sizes at least.
> 
> I might perfectly be wrong with my guess. But at least it's a start (for 
> the next step into advancing the test).
> 
> S.
> 
> [1] 
> https://github.com/LordNoteworthy/windows-internals/blob/master/windows-internals-6th-ed.md#user-address-space-layout 
> 
> [2] 
> https://docs.microsoft.com/en-us/gaming/playfab/features/multiplayer/servers/determining-required-dlls#:~:text=the%20lists%20are%20the%20system%20DLLs 
> 
> [3] https://www.nirsoft.net/utils/cprocess.html
> [4] 
> https://blog.malwarebytes.com/threat-analysis/2013/06/my-memory-isnt-what-it-used-to-be-part-1/#:~:text=files%20seen%20in%20the%20memory%20map 
> 
> [5] 
> https://www.cyberark.com/resources/threat-research-blog/masking-malicious-memory-artifacts-part-i-phantom-dll-hollowing-2#:~:text=Figure%202.,memory 
> 
> [6] 
> https://www.codeproject.com/Articles/86215/Remote-Threads-Basics-Part-1#:~:text=address%20of%20any%20function%20can,and%20function%20offset 
> 
> 

This is overly complicating things. The current test checks if it's 
within 256 bytes to the test function's address, so it's already testing 
everything you listed (almost). Only downside is that an arbitrary range 
(256) since AFAIK we can't use assembly code in tests. If we could, we 
could easily test it exactly.