[Bug 46754] New: Multiple application installers fail in VBscripts due to IWshEnvironment: :Item property returning BSTR with embedded null terminator ( PostgreSQL 9.3)

https://bugs.winehq.org/show_bug.cgi?id=46754

Bug ID: 46754
           Summary: Multiple application installers fail in VBscripts due
                    to IWshEnvironment::Item property returning BSTR with
                    embedded null terminator (PostgreSQL 9.3)
           Product: Wine
           Version: 4.3
          Hardware: x86-64
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: wshom.ocx
          Assignee: [e-mail]
          Reporter: [e-mail]
      Distribution: ---

Hello folks,

as it says. Found while revisiting bug 37972

Trace log:

--- snip ---
$ WINEDEBUG=+seh,+relay,+wshom,+vbscript,+ole,+variant wine
./postgresql-9.3.5-3-windows.exe >>log.txt 2>&1

...
002c:trace:wshom:WshEnvironment_Invoke (0x1706c0)->(0
{00000000-0000-0000-0000-000000000000} 1033 3 0x33facc 0x33fb30 0x33fa00
0x33f9fc)
...
002c:trace:ole:dump_DispParms args=1 named args=0
002c:trace:ole:dump_DispParms args:
002c:trace:ole:dump_DispParms   [0] 0x167848 {VT_BSTR: L"TEMP"}
002c:trace:ole:ITypeInfo_fnInvoke invoking:
L"Item"(2)
    parm0: L"Name"
    parm1: L"out_Value"
memid is 00000000
Param 0:
        tdesc.vartype 8 (VT_BSTR)
        u.paramdesc.wParamFlags PARAMFLAG_FIN
        u.paramdesc.lpex (nil)
Param 1:
        tdesc.vartype 26 (ptr to VT_BSTR)
        u.paramdesc.wParamFlags PARAMFLAG_FOUT PARAMFLAG_FRETVAL
        u.paramdesc.lpex (nil)
    funckind: 1 (pure virtual)
    invkind: 2 (property get)
    callconv: 4 (stdcall)
    oVft: 28
    cParamsOpt: 0
    wFlags: 0
    elemdescFunc (return value type):
        tdesc.vartype 25 (VT_HRESULT)
        u.paramdesc.wParamFlags PARAMFLAGS_NONE
        u.paramdesc.lpex (nil)
    helpstring: (null)
    entry: invalid
...
002c:trace:ole:ITypeInfo_fnInvoke changing args
002c:trace:ole:ITypeInfo_fnInvoke 0x167848 {VT_BSTR: L"TEMP"}
002c:trace:ole:DispCallFunc (0x1706cc, 28, 4, 10, 2, 0x170680, 0x170678,
0x33f6b0 (vt=10))
...
002c:trace:ole:DispCallFunc arg 0: type VT_BSTR 0x167848 {VT_BSTR: L"TEMP"}
002c:trace:ole:DispCallFunc arg 1: type VT_BSTR|VT_BYREF 0x170648
{VT_BSTR|VT_BYREF: (null)}
002c:trace:wshom:WshEnvironment_get_Item (0x1706c0)->(L"TEMP" 0x33f6a0)
002c:Call KERNEL32.GetEnvironmentVariableW(00165fcc L"TEMP",00000000,00000000)
ret=f781953d
002c:Ret  KERNEL32.GetEnvironmentVariableW() retval=00000014 ret=f781953d
002c:Call oleaut32.SysAllocStringLen(00000000,00000014) ret=f7819550
002c:trace:ole:SysAllocStringLen (null)
002c:Ret  oleaut32.SysAllocStringLen() retval=00167954 ret=f7819550
002c:Call KERNEL32.GetEnvironmentVariableW(00165fcc L"TEMP",00167954,00000015)
ret=f7819588
002c:Ret  KERNEL32.GetEnvironmentVariableW() retval=00000013 ret=f7819588
...
002c:trace:ole:DispCallFunc retval: 0x33f6b0 {VT_ERROR: 00000000}
...
002c:trace:ole:ITypeInfo_fnInvoke [retval] value: 0x170648 {VT_BSTR|VT_BYREF:
L"C:\users\focht\Temp"}
002c:trace:variant:VariantInit (0x33fb30)
002c:trace:variant:VariantCopyInd (0x33fb30 {VT_EMPTY},0x170648
{VT_BSTR|VT_BYREF: L"C:\users\focht\Temp"})
002c:trace:variant:VariantClear (0x33fb30 {VT_EMPTY})
...
002c:trace:variant:VariantCopyInd returning 0x00000000, 0x33fb30 {VT_BSTR:
L"C:\users\focht\Temp"}
002c:trace:variant:VARIANT_ClearInd (0x170648 {VT_BSTR|VT_BYREF:
L"C:\users\focht\Temp"})
...
002c:trace:ole:ITypeInfo_fnInvoke -- 0x00000000
002c:trace:ole:ITypeInfo_fnRelease (0x16dbc8)->(1)
...
002c:trace:vbscript:interp_assign_ident L"strSystemRoot"
...
002c:trace:vbscript:assign_ident creating variable L"strSystemRoot"
...
002c:Call oleaut32.VariantCopyInd(00170720,00167848) ret=f7858cba
002c:trace:variant:VariantCopyInd (0x170720 {VT_EMPTY},0x167848 {VT_BSTR:
L"C:\users\focht\Temp"})
002c:trace:variant:VariantCopy (0x170720 {VT_EMPTY},0x167848 {VT_BSTR:
L"C:\users\focht\Temp"})
002c:trace:variant:VariantClear (0x170720 {VT_EMPTY})
002c:Ret  oleaut32.VariantCopyInd() retval=00000000 ret=f7858cba
002c:Call oleaut32.VariantClear(00167848) ret=f7857d74
002c:trace:variant:VariantClear (0x167848 {VT_BSTR: L"C:\users\focht\Temp"})
...
002c:Ret  oleaut32.VariantClear() retval=00000000 ret=f7857d74
002c:trace:vbscript:interp_icall 
002c:trace:vbscript:interp_string 
002c:Call oleaut32.SysAllocString(00165b38 L"") ret=f785a7f5
002c:trace:ole:SysAllocStringLen L""
002c:Ret  oleaut32.SysAllocString() retval=001660f4 ret=f785a7f5
002c:trace:vbscript:interp_equal 
002c:trace:vbscript:var_cmp 0x170720 {VT_BSTR: L"C:\users\focht\Temp"}
0x167858 {VT_BSTR: L""}
002c:Call oleaut32.VarCmp(00170720,00167858,00000409,00000000) ret=f785b019
002c:trace:variant:VarCmp (0x170720 {VT_BSTR:
L"C:\users\focht\Temp"},0x167858 {VT_BSTR: L""},0x00000409,0x00000000)
...
002c:trace:variant:VarBstrCmp L"C:\users\focht\Temp\0000",L"",1033,00000000
002c:Ret  oleaut32.VarCmp() retval=00000002 ret=f785b019
...
002c:Call oleaut32.VarCat(00170720,00170830,0033f950) ret=f785b870
002c:trace:variant:VarCat 0x170720 {VT_BSTR:
L"C:\users\focht\Temp"},0x170830 {VT_BSTR: L"\"},0x33f950)
002c:trace:variant:VarBstrCat L"C:\users\focht\Temp\0000",L"\",0x33f958
002c:trace:variant:VarBstrCat L"C:\users\focht\Temp\0000\"
...
002c:Call KERNEL32.CreateFileW(0017ca4c
L"C:\users\focht\Temp",40000000,00000000,00000000,00000002,00000080,00000000)
ret=f77e4bfd
002c:Ret  KERNEL32.CreateFileW() retval=ffffffff ret=f77e4bfd
...
002c:warn:ole:ITypeInfo_fnInvoke invoked function failed with error 0x800a0046 
...
002c:trace:vbscript:assign_ident creating variable L"isWritable" 
...
Unable to write inside TEMP environment variable path
...
--- snip ---

The VBscript snippet failing:

--- snip ---
...
Set WshShell = WScript.CreateObject("WScript.Shell")
Set WshSysEnv = WshShell.Environment("PROCESS")
strSystemRoot = WshSysEnv("TEMP")

If strSystemRoot = "" Then
   WScript.Echo "Unable to read the value of the TEMP environment variable."
   WScript.Quit 1
End If

' Check if temp path is writable
isWritable = IsPathWriteable(strSystemRoot)
If isWritable = False Then
   WScript.Echo "Unable to write inside TEMP environment variable path."
   WScript.Quit 1
End If

...

Function IsPathWriteable(Path)' As Boolean
    Dim fso 'As Scripting.FileSystemObject
    Dim Temp_Path 'As String
    Set fso = CreateObject("Scripting.FileSystemObject")
    Temp_Path = Path & "" & fso.GetTempName() & ".tmp"
    On Error Resume Next
    fso.CreateTextFile Temp_Path
    IsPathWriteable = Err.Number = 0
    fso.DeleteFile Temp_Path
    On Error Goto 0
End Function 
--- snip ---

Wine source:

https://source.winehq.org/git/wine.git/blob/979f9078d3a2c4406dc494c81b86fea2...

--- snip ---
 447 static HRESULT WINAPI WshEnvironment_get_Item(IWshEnvironment *iface, BSTR
name, BSTR *value)
 448 {
 449     WshEnvironment *This = impl_from_IWshEnvironment(iface);
 450     DWORD len;
 451 
 452     TRACE("(%p)->(%s %p)\n", This, debugstr_w(name), value);
 453 
 454     if (!value)
 455         return E_POINTER;
 456 
 457     len = GetEnvironmentVariableW(name, NULL, 0);
 458     *value = SysAllocStringLen(NULL, len);
 459     if (!*value)
 460         return E_OUTOFMEMORY;
 461 
 462     if (len)
 463         GetEnvironmentVariableW(name, *value, len+1);
 464 
 465     return S_OK;
 466 }
--- snip ---

Microsoft docs:

https://docs.microsoft.com/en-us/windows/desktop/api/winbase/nf-winbase-gete...

--- quote ---
Return Value

If the function succeeds, the return value is the number of characters stored
in the buffer pointed to by lpBuffer, not including the terminating null
character.

If lpBuffer is not large enough to hold the data, the return value is the
buffer size, in characters, required to hold the string and its terminating
null character and the contents of lpBuffer are undefined.
--- quote ---

When you use SysAllocStringLen() you have to ensure the null terminator is
excluded from number of characters. Also the second GetEnvironmentVariableW()
call doesn't need +1 as the first call already returns the proper count (len)
which includes the null terminator.

The faulty code was introduced in
https://source.winehq.org/git/wine.git/commitdiff/ac65c3fb18d9c9be4aa43f373b...
("wshom.ocx: Implement get_Item() for IWshEnvironment.")

$ sha1sum postgresql-9.3.5-3-windows.exe 
e47b6569f86466da76199186aa1b63a94f040551  postgresql-9.3.5-3-windows.exe

$ du -sh postgresql-9.3.5-3-windows.exe 
53M    postgresql-9.3.5-3-windows.exe

$ wine --version
wine-4.3

Regards