[PATCH v2] ntdll: Fix overflow if running 32bit app w/ wine64

[Brendan McGrath]

I'm following up on this to patch to better illustrate the problem.

If you run the bash script I have included below - it will create two 
applications:
1. mono32.exe: PE32 executable (console) Intel 80386 Mono/.Net assembly, 
for MS Windows; and
2. c64.exe:    PE32+ executable (console) x86-64, for MS Windows

mono32 being a 32bit mono application and c64 being a 64bit 'C' 
application (this emulates the set-up of the launcher and application 
I'm trying to get working).

When ran, mono32 grabs the MachineGuid from the registry. It then passes 
it to c64.exe, which grabs the same registry value and compares them.

These executables work under Windows but do not work under Wine. This is 
because if you run:
wine mono32

mono32 will get a different value to c64. This is because 'wine mono32' 
will fetch the value from the WoW6432 branch. This currently has a 
different value to the 64bit branch.

(I have already submitted a patch to ensure these values are the same - 
but it was rightly rejected. As Nikolay Sivov pointed out - Windows 
doesn't have a WoW6432 entry and neither should Wine. Another patch has 
been submitted for that).

But if you run:
wine64 mono32

you get the Out of Memory error that is fixed by this patch. Applying 
this patch allows these applications to work as they currently do under 
Windows.

So apart from manually copying the 64bit MachineGuid entry to the 
WoW6432 branch, then without this patch, I'm not aware of any other 
solution. Please let me know if there is one.

Below is the bash script that demonstrates the issue (you can just copy 
and paste the whole lot):

# create mono source
cat << 'END' > mono32.cs
using Microsoft.Win32;
using System.Diagnostics;
using System.Reflection;
using System.IO;
using System;

public class Launcher
{
     static public void Main ()
     {
         RegistryKey rk = 
Registry.LocalMachine.OpenSubKey("SOFTWARE\\Microsoft\\Cryptography");

         if (rk != null) {
            object guid = rk.GetValue("MachineGuid");
            if (guid != null) {
               string strGuid = guid.ToString();
               string cwd = 
Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location);
               ProcessStartInfo processStartInfo = new ProcessStartInfo();
               processStartInfo.FileName = cwd + "\\c64.exe";
               processStartInfo.Arguments = strGuid;
               processStartInfo.UseShellExecute = false;
               Process.Start(processStartInfo);
            } else {
               Console.WriteLine ("guid not found - run under 'wine64'");
            }
         } else {
            Console.WriteLine("Mono couldn't find 
'HKLM\\SOFTWARE\\Microsoft\\Cryptography'");
         }
     }
}
END

# create 'C' source
cat << 'END' > c64.c
#include <windows.h>
#include <stdio.h>

int main(int argc, const char *argv[])
{
     LONG rc;
     BYTE data[200];
     DWORD data_size = 200;
     HKEY hkey;

     if (argc != 2)
         printf("Usage: c64.exe guid\n");

     if (RegOpenKeyExA(HKEY_LOCAL_MACHINE, 
"Software\\Microsoft\\Cryptography", 0, KEY_QUERY_VALUE, &hkey) == 
ERROR_SUCCESS) {
         rc = RegQueryValueExA(hkey, "MachineGuid", NULL, NULL, data, 
&data_size);
         if (rc == ERROR_SUCCESS) {
             if (strcmp(data, argv[1]) == 0) {
                 printf("We match - I work\n");
             } else {
                 printf("We don't match. I got:\n%s\nMono gave 
me:\n%s\n", data, argv[1]);
             }
         } else {
             printf("guid not found\n", rc);
         }
     } else {
         printf("c64 couldn't find 
'HKLM\\SOFTWARE\\Microsoft\\Cryptography'\n");
     }
}
END

# compile mono
mcs mono32.cs

# compile 'C'
x86_64-w64-mingw32-gcc c64.c -o c64.exe

# try wine (always fails as the guid is wrong or missing)
wine mono32

# try wine64 (fails with out of memory unless the overflow patch is applied)
wine64 mono32

On 17/10/18 8:49 pm, Brendan McGrath wrote:
> wine64 was using IMAGE_NT_HEADERS to access header information
> regardless of the execution type; hence it would use
> IMAGE_OPTIONAL_HEADER64 for a 32bit app.
>
> This could result in an overflow and a request to mmap for a huge amount
> of memory causing an out of memory error.
>
> This patch ensures IMAGE_OPTIONAL_HEADER32 is used for a 32-bit app
> and IMAGE_OPTIONAL_HEADER64 is used for a 64-bit app
>
> Signed-off-by: Brendan McGrath <brendan at redmandi.com>
> ---
>
> Fixed the formatting - sorry about that
>
>   dlls/ntdll/virtual.c | 23 ++++++++++++++++++++---
>   1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/dlls/ntdll/virtual.c b/dlls/ntdll/virtual.c
> index af1509eae5d..a05000f2e2b 100644
> --- a/dlls/ntdll/virtual.c
> +++ b/dlls/ntdll/virtual.c
> @@ -1933,9 +1933,26 @@ NTSTATUS virtual_alloc_thread_stack( TEB *teb, SIZE_T reserve_size, SIZE_T commi
>   
>       if (!reserve_size || !commit_size)
>       {
> -        IMAGE_NT_HEADERS *nt = RtlImageNtHeader( NtCurrentTeb()->Peb->ImageBaseAddress );
> -        if (!reserve_size) reserve_size = nt->OptionalHeader.SizeOfStackReserve;
> -        if (!commit_size) commit_size = nt->OptionalHeader.SizeOfStackCommit;
> +        struct nt
> +        {
> +            DWORD Signature;
> +            IMAGE_FILE_HEADER FileHeader;
> +            union
> +            {
> +                IMAGE_OPTIONAL_HEADER32 hdr32;
> +                IMAGE_OPTIONAL_HEADER64 hdr64;
> +            } opt;
> +        };
> +
> +        struct nt *nt = (struct nt*) RtlImageNtHeader( NtCurrentTeb()->Peb->ImageBaseAddress );
> +        if (nt->opt.hdr32.Magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> +            if (!reserve_size) reserve_size = nt->opt.hdr32.SizeOfStackReserve;
> +            if (!commit_size) commit_size = nt->opt.hdr32.SizeOfStackCommit;
> +        } else {
> +            if (!reserve_size) reserve_size = nt->opt.hdr64.SizeOfStackReserve;
> +            if (!commit_size) commit_size = nt->opt.hdr64.SizeOfStackCommit;
> +        }
> +        TRACE("reserve_size: %lu, commit_size: %lu", reserve_size, commit_size);
>       }
>   
>       size = max( reserve_size, commit_size );