Application Verifier reports access violation in call to ShellExecuteEx

Question

Short Version

Application Verifier says there is an access violation when running the code:

var
   shi: TShellExecuteInfo;
begin
   shi := Default(TShellExecuteInfo);
   shi.cbSize := SizeOf(TShellExecuteInfo);
   shi.lpFile := PChar('C:\Windows');
   ShellExecuteEx(@shi);
end;

What's wrong with it?

Long Version

I'm running my application under the Application Verifier, with the option to detect heap corruption enabled:

• Heaps: Checks the heap errors.

During the call to ShellExecuteEx, an exception comes up, indicating that there is heap corruption.

Running inside a debugger allows me to decode the exception:

ExceptionAddress: 0000000074b254ad (KERNELBASE!ParseURLW+0x000000000000002d)
   ExceptionCode: c0000005 (Access violation)
  ExceptionFlags: 00000000
NumberParameters: 2
   Parameter[0]: 0000000000000000
   Parameter[1]: 0000000008e26fe8
Attempt to read from address 0000000008e26fe8

Running outside a debugger, the application crashes (WerFault takes a post-mortem and the process dies).

What is wrong with the code?

Example

program ShellExecuteTestApp;

{$APPTYPE CONSOLE}

uses
  System.SysUtils,
  Winapi.ShellAPI;

var
    shi: TShellExecuteInfo;
begin
    try
        shi := Default(TShellExecuteInfo);
        shi.cbSize := SizeOf(TShellExecuteInfo);
        shi.lpFile := PChar('C:\Windows');

        WriteLn('shi.cbSize: '+IntToStr(shi.cbSize));
        WriteLn('shi.lpFile: "'+shi.lpFile+'"');
        ShellExecuteEx(@shi);
    except
        on E: Exception do
            Writeln(E.ClassName, ': ', E.Message);
    end;
end.

Before crashing, it outputs:

shi.cbSize: 60
shi.lpFile: "C:\Windows"

I thought perhaps it was the common bug from CreateProcess, so i made sure the lpFile was writable:

var
   file: string;
   shi: TShellExecuteInfo;
begin
   file := 'C:\Windows';
   UniqueString({var} file);

   shi := Default(TShellExecuteInfo);
   shi.cbSize := SizeOf(TShellExecuteInfo);
   shi.lpFile := PChar(file);
   ShellExecuteEx(@shi);
end;

Memory dumps

Using a 32-bit build (to keep pointers easier to read), the memory being passed to ShellExecuteEx

00423EBC   0000003C  cbSize         60 bytes
           00000000  fMask          SEE_MASK_DEFAULT
           00000000  Wnd            null
           00000000  lpVerb         null
           0041C600  lpFile         ==> 0x0041C60C
           00000000  lpParameters   null
           00000000  lpDirectory    null 
           00000001  nShow          SW_NORMAL
           00000000  hInstApp       0
           00000000  lpIDList       null
           00000000  lpClass        null
           00000000  hkeyClass      0
           00000000  dwHotKey       0
           00000000  hMonitor       null
           00000000  hProcess       null

With the one pointer to a wide char:

0041C60C  43 00 3A 00 5C 00 57 00  C.:.\.W.
0041C614  69 00 6E 00 64 00 6F 00  i.n.d.o.
0041C61C  77 00 73 00 00 00 00 00  w.s.....

It then calls:

ShellExecuteTestApp.dpr.29: ShellExecuteEx(@shi);
0041C51F 68BC3E4200       push $00423ebc       ;push address of shi structure
0041C524 E85BD6FFFF       call ShellExecuteEx  ;call imported function

Winapi.ShellAPI.pas.1798: function ShellExecuteEx; external shell32 name 'ShellExecuteExW';
00419B84 FF250C444200     jmp dword ptr [$0042440c] ;jump to ShellExecuteW inside shell32


shell32.ShellExecuteExW:
75520060 8BFF             mov edi,edi
75520062 55               push ebp
75520063 8BEC             mov ebp,esp
75520065 83E4F8           and esp,-$08
75520068 51               push ecx
75520069 53               push ebx
7552006A 56               push esi
7552006B 8B7508           mov esi,[ebp+$08]
7552006E 57               push edi
7552006F 833E3C           cmp dword ptr [esi],$3c
75520072 7540             jnz $755200b4
75520074 8B5E04           mov ebx,[esi+$04]
75520077 F7C300011000     test ebx,$00100100
7552007D 7427             jz $755200a6
7552007F 8BCE             mov ecx,esi
75520081 E836000000       call $755200bc --> failure is this way

And then there's a lot of stuff i can't follow, until the error finally happens in ParseUrlW:

HRESULT ParseURL(
  _In_    LPCTSTR   pcszUrl,
  _Inout_ PARSEDURL *ppu
);

with dump:

KERNELBASE.ParseURLW:
74B25480 8BFF             mov edi,edi
74B25482 55               push ebp
74B25483 8BEC             mov ebp,esp         ;save stack pointer
74B25485 8B4508           mov eax,[ebp+$08]   ;restore pcszUrl into EAX
74B25488 83EC0C           sub esp,$0c
74B2548B 85C0             test eax,eax        ;test that pcszUrl param supplied
74B2548D 0F8493010000     jz $74b25626

---

Note: At this moment EAX contains address 0x0908BFE8:

0908BFE8: 00 00 00 00 00 00 00 00  ........

The supplied pcszUrl string is empty? Actually no, the address is invalid. But we don't know that yet until the code tries to touch it

---

74B25493 57               push edi            ;save EDI
74B25494 8B7D0C           mov edi,[ebp+$0c]   ;get PARSEDURL into edi
74B25497 85FF             test edi,edi
74B25499 0F8410630300     jz $74b5b7af

---

Note: At this moment edi contains 0977FBDC

PARSEDURL structure
0977FBCD   00000018   cbSize      24 bytes
           753F5BC0   pszProtocol [uninitialized junk]
           0977FEC4   cchProtocol [uninitialized junk]
           092ECFC8   pszSuffix   [uninitialized junk]
           78E6E41E   cchSuffix   [uninitialized junk]
           0977FE98   nScheme     [uninitialized junk]

---

74B2549F 833F18           cmp dword ptr [edi],$18  ;test that struct size is what we expect (24 bytes)
74B254A2 0F8507630300     jnz $74b5b7af

74B254A8 53               push ebx           ;save ebx
74B254A9 8BD0             mov edx,eax        ;save pcszUrl into edx
74B254AB 33DB             xor ebx,ebx     

74B254AD 0FB700           movzx eax,[eax]    ;Attempt to copy 4 bytes of the string into EAX (access violation)

The code tries to copy the first word from 0x0908BFE8 using Move with Zero-Extend. But that address is invalid, giving the access violation:

access violation at 0x74b254ad: read of address 0x0908bfe8

So somewhere in the code of ShellExecuteExW, it sets up an invalid call to ParseUrlW.

WinDBG

32-bit WinDbg, debugging 32-bit application, with symbols:

*******************************************************************************
*                                                                             *
*                        Exception Analysis                                   *
*                                                                             *
*******************************************************************************

*** ERROR: Module load completed but symbols could not be loaded for image00400000

FAULTING_IP: 
KERNELBASE!ParseURLW+2d
74b254ad 0fb700          movzx   eax,word ptr [eax]

EXCEPTION_RECORD:  ffffffff -- (.exr 0xffffffffffffffff)
ExceptionAddress: 74b254ad (KERNELBASE!ParseURLW+0x0000002d)
   ExceptionCode: c0000005 (Access violation)
  ExceptionFlags: 00000000
NumberParameters: 2
   Parameter[0]: 00000000
   Parameter[1]: 07634fe8
Attempt to read from address 07634fe8

CONTEXT:  00000000 -- (.cxr 0x0;r)
eax=07634fe8 ebx=00000000 ecx=07634fe8 edx=07634fe8 esi=00000000 edi=093dfbdc
eip=74b254ad esp=093dfbb4 ebp=093dfbc8 iopl=0         nv up ei pl zr na pe nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010246
KERNELBASE!ParseURLW+0x2d:
74b254ad 0fb700          movzx   eax,word ptr [eax]       ds:002b:07634fe8=????

FAULTING_THREAD:    00003ff4
DEFAULT_BUCKET_ID:  INVALID_POINTER_READ
PROCESS_NAME:       image00400000
ERROR_CODE:         (NTSTATUS) 0xc0000005 - The instruction at 0x%p referenced memory at 0x%p. The memory could not be %s.
EXCEPTION_CODE:     (NTSTATUS) 0xc0000005 - The instruction at 0x%p referenced memory at 0x%p. The memory could not be %s.
EXCEPTION_PARAMETER1:  00000000
EXCEPTION_PARAMETER2:  07634fe8
READ_ADDRESS:       07634fe8 

FOLLOWUP_IP: 
shell32!GetUrlSchemeW+28
754c66be 85c0            test    eax,eax

NTGLOBALFLAG:                2000100
APPLICATION_VERIFIER_FLAGS:  80000001
APP:                         image00400000
ANALYSIS_VERSION:            6.3.9600.17237 (debuggers(dbg).140716-0327) x86fre
PRIMARY_PROBLEM_CLASS:       INVALID_POINTER_READ
BUGCHECK_STR:                APPLICATION_FAULT_INVALID_POINTER_READ
LAST_CONTROL_TRANSFER:       from 754c66be to 74b254ad

STACK_TEXT:  
093dfbc8 754c66be 07634fe8 093dfbdc 00000000 KERNELBASE!ParseURLW+0x2d
093dfbf8 75522df8 0746fff0 07604f68 00000002 shell32!GetUrlSchemeW+0x28
093dfe98 75522b1a 093dfec4 00000000 0746fff0 shell32!CShellExecute::CreateParsingBindCtx+0x1d6
093dfecc 755224da 00000000 0746fff0 00000000 shell32!CShellExecute::ParseOrValidateTargetIdList+0x37
093dfef0 7551fd5a 093dff80 768b983a 07604f68 shell32!CShellExecute::_DoExecute+0x40
093dfef8 768b983a 07604f68 768b9770 768b9770 shell32!<lambda_e76b82c5cb7f9f82cbe0fd97ad5190bf>::<lambda_invoker_stdcall>+0x1a
093dff80 73d08484 0019fd94 73d08460 21439000 shcore!_WrapperThreadProc+0xca
093dff94 772c2fea 0019fd94 25ae5778 00000000 KERNEL32!BaseThreadInitThunk+0x24
093dffdc 772c2fba ffffffff 772dec22 00000000 ntdll!__RtlUserThreadStart+0x2f
093dffec 00000000 768b9770 0019fd94 00000000 ntdll!_RtlUserThreadStart+0x1b


STACK_COMMAND:       .cxr 0x0 ; kb
SYMBOL_STACK_INDEX:  1
SYMBOL_NAME:         shell32!GetUrlSchemeW+28
FOLLOWUP_NAME:       MachineOwner
MODULE_NAME:         shell32
IMAGE_NAME:          shell32.dll
DEBUG_FLR_IMAGE_TIMESTAMP:  0
FAILURE_BUCKET_ID:   INVALID_POINTER_READ_c0000005_shell32.dll!GetUrlSchemeW
BUCKET_ID:     
APPLICATION_FAULT_INVALID_POINTER_READ_shell32!GetUrlSchemeW+28
ANALYSIS_SOURCE:     UM
FAILURE_ID_HASH_STRING:  um:invalid_pointer_read_c0000005_shell32.dll!geturlschemew
FAILURE_ID_HASH:     {89d9bcf0-5ef6-4e90-df6b-f05dc028e062}
Followup:            MachineOwner
---------

Bonus Reading

• Access Violation ShellExecute in delphi7

Granulose

Your code is fine. I suspect that ShellExecuteEx is trying to read one of the strings that is permitted to be NULL. Try setting them to be '' until the verifier is happy. Or look at the address that the verifier returns to work out which field is being accessed with a first chance exception. – Escribe

First-chance exceptions are not the exceptions you should worry about, unless they are followed by a second-chance exception. If all you see is a first-chance exception, then someone was prepared for it, and handled it already. Not unlikely for ShellExecuteEx, with all the foreign code it runs while servicing your request. You should probably have a look at address 0x74B254AD to find the faulting module. – Monroe

@Monroe I can turn off first-chance exceptions, leaving on detection of Heap Corruption. I then no longer see (helpful) first chance exceptions, and instead get a lovely crash when the code violates the heap in a most inappropriate manner. – Granulose

Do you have reason to believe, that both events are related? – Monroe

The detection of heap corruption throws an exception. The exception is unhandled, and will crash the application. The first-chance exception option gives me informative exception information as an OutputDebugString. Alternatively i can disable detailing reporting of first chance exceptions, and let the application crash. I can then use the post-mortem debugger to get the same information that the First Chance Exception info gave me. *tl;dr: The first-chance exception option is a red herring, and i shall remove references to it in the question to avoid confusing people. – Granulose

Maybe you should call CoInitializeEx before calling ShellExecuteEx? Also, does the AV happen in your code, might be some shell extension etc? – Planetesimal

You didn't initialize many of the members of shi. Perhaps ShellExecute is picking up uninitialized garbage? (I see other people explicitly clearing the TShellExecuteInfo.) – Connett

@Raymond The assignment of Default(...) zero initialized the record. All members are initialized. – Escribe

Okay, thanks for explaining. I know Pascal but not Delphi. Can you share a stack trace (and OS version information)? Or you can dig the Watson failure ID out of the event log and I can try to hunt for it on the back-end. – Connett

@RaymondChen I added some dumps and traced somwhat into ShellExecuteExW; the failure happens internally during the call the ParseUrlW where it supplies an invalid address to the pcszUrl. It seems to succeed if i add a trailing backslash; change the filename from "C:\Windows" to `"C:\Windows\" – Granulose

Link to test app binary – Granulose

@RaymondChen Report Id: a8a75eb7-d024-4d0b-9f2d-d037368a8ae0. Windows 10.0.17134 Build 17134. – Granulose

Zipped mini-dump – Granulose

Somehow I couldn't download the mini-dump, but the report ID gave me access to the Watson crash dump. This is a use-after free bug in the OS that occurs if we had to fiddle with the name. It looks like you have a workaround (add the trailing backslash); I'll file a bug to get this fixed in Windows. Thanks again for your patience. – Connett

@RaymondChen. Thanks! Bonus: Time travel debugging trace zip (i can't imagine anyplace else to put them that your IT would allow) – Granulose

Answer 1

As @RaymondChen said, it's a bug in Windows 10.

• I only encountered it because Application Verifier caught it.
• it's a one-in-a-million chance that you'd actually ever encounter it in the wild

Raymond's too modest to accept reputation; so i use his answer as my own.

Granulose

Answer 2

A code that has been running OK for years started to fail with an Access Violation in KernelBase.dll. I think that some Win10 upgrade must have changed something on that dll.

I found this Question and thanks to the comment of @David Heffernan I added the line

ExecInfo.lpParameters := ''; 

and that solved the problem.

var
  ExecInfo: TShellExecuteInfo;  
begin
  ExecInfo.cbSize := SizeOf(ExecInfo);
  ExecInfo.fMask := SEE_MASK_NOCLOSEPROCESS;
  ExecInfo.Wnd := 0; // Handle;
  ExecInfo.lpVerb := 'runas';    
  ExecInfo.lpFile := PChar(sPath + sFileName);
  ExecInfo.lpDirectory := PChar(sPath);
  ExecInfo.nShow := SW_HIDE; 
  ExecInfo.lpParameters := '';  //PJR@20210125  Added this line to avoid AccessViolation in KernelBase.dll due to a bug in some version of Win10.

  if ShellExecuteEx(@ExecInfo) then
    (...)

Rhodarhodamine

The issue in this code that nobody is initializing ExecInfo. Anything not defined is undefined, and you cannot pass undefined parameters to a function. It's one of the basic rules of programming. The old-fashioned way is to call ZeroMemory(@execInfo, sizeof(execInfo)). – Granulose