[PATCH v2 0/1] MR8639: rpcrt4: Don't read past the end of params in client_do_args.

[Yuxuan Shui (＠yshui)]

* * *

An example is `IDispatchEx_RemoteInvokeEx_Proxy`

asan complaint:

```
015c:0160:err:asan:asan_report ASan: read of 4 bytes at 015A88DC, caller 6A9A0EFA (__asan_report_load4_noabort, /home/shui/programs/wine/dlls/asan_dynamic_thunk/thunk.c:1031,1)
015c:0160:err:asan:asan_report stacktrace:
015c:0160:err:asan:asan_report 	6A9A0EFA (__asan_report_load4_noabort, /home/shui/programs/wine/dlls/asan_dynamic_thunk/thunk.c:1031,1)
015c:0160:err:asan:asan_report 	6A9ACB2C (client_do_args, /home/shui/programs/wine/dlls/rpcrt4/ndr_stubless.c:511,17)
015c:0160:err:asan:asan_report 	6A9C5913 (ndr_client_call, /home/shui/programs/wine/dlls/rpcrt4/ndr_stubless.c:794,9)
015c:0160:err:asan:asan_report 	6A965438 (NdrpClientCall2, /home/shui/programs/wine/dlls/rpcrt4/ndr_stubless.c:929,22)
015c:0160:err:asan:asan_report 	6A916C77 (NdrClientCall2)
015c:0160:err:asan:asan_report 	68662856 (IDispatchEx_RemoteInvokeEx_Proxy, dlls/dispex/i386-windows/disp_ex_p.c:82,15)
015c:0160:err:asan:asan_report 	68661D6E (IDispatchEx_InvokeEx_Proxy, /home/shui/programs/wine/dlls/dispex/usrmarshal.c:100,10)
015c:0160:err:asan:asan_report 	00401187 (IDispatchEx_InvokeEx, include/dispex.h:319,12)
015c:0160:err:asan:asan_report 	00408F36 (test_dispex, /home/shui/programs/wine/dlls/dispex/tests/marshal.c:391,10)
015c:0160:err:asan:asan_report 	004061FF (func_marshal, /home/shui/programs/wine/dlls/dispex/tests/marshal.c:433,5)
015c:0160:err:asan:asan_report 	00407B60 (run_test, /home/shui/programs/wine/include/wine/test.h:794,9)
015c:0160:err:asan:asan_report 	00407426 (main, /home/shui/programs/wine/include/wine/test.h:912,1)
015c:0160:err:asan:asan_report 	0040767C (mainCRTStartup, /home/shui/programs/wine/dlls/msvcrt/crt_main.c:60,11)
015c:0160:err:asan:asan_report 	7901FA24 (BaseThreadInitThunk)
015c:0160:err:asan:asan_report 	7A50E0F7 (call_thread_func_wrapper)
015c:0160:err:asan:asan_report 	7A5B6ADD (call_thread_func, /home/shui/programs/wine/dlls/ntdll/signal_i386.c:505,5)
015c:0160:err:asan:asan_report info:
015c:0160:err:asan:asan_report 	partial granule: 4
015c:0160:err:asan:asan_report 	stack-buffer-overflow, addr is 4 bytes to the right of the end of stack
015c:0160:err:asan:asan_report 	stack: [015A8820, 015A8860)
015c:0160:err:asan:asan_report 	stack pc: 6866252C (IDispatchEx_RemoteInvokeEx_Proxy, dlls/dispex/i386-windows/disp_ex_p.c:54,1), descr: 2 32 4 10 _RetVal:55 48 44 11 __params:69
```

the parameters info:

```
0024:trace:rpc:ndr_client_call INITOUT
0024:trace:rpc:client_do_args param[0]: 015EB834 type 08 IsIn IsBasetype
0024:trace:rpc:client_do_args param[1]: 015EB838 type 08 IsIn IsBasetype
0024:trace:rpc:client_do_args param[2]: 015EB83C type 08 IsIn IsBasetype
0024:trace:rpc:client_do_args param[3]: 015EB840 type 16 MustSize MustFree IsIn IsSimpleRef
0024:trace:rpc:client_do_args param[4]: 015EB844 type b4 MustSize MustFree IsOut IsSimpleRef ServerAllocSize = 16
0024:trace:rpc:client_do_args param[5]: 015EB848 type 1a MustSize MustFree IsOut IsSimpleRef ServerAllocSize = 32
0024:trace:rpc:client_do_args param[6]: 015EB84C type 2f MustSize MustFree IsIn
0024:trace:rpc:client_do_args param[7]: 015EB850 type 08 IsIn IsBasetype
0024:trace:rpc:client_do_args param[8]: 015EB854 type 1b MustSize MustFree IsIn IsSimpleRef
0024:trace:rpc:client_do_args param[9]: 015EB858 type 21 MustSize MustFree IsIn IsOut IsSimpleRef
0024:trace:rpc:client_do_args param[10]: 015EB85C type 08 IsOut IsReturn IsBasetype
```

reading of `param[10]` at `015EB85C`, which is the non-existent `_RetVal`, triggers the asan report.

--
  v2: rpcrt4: Don't read past the end of params in client_do_args.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8639

[PATCH v2 1/1] rpcrt4: Don't read past the end of params in client_do_args.

[Yuxuan Shui]

From: Yuxuan Shui <yshui(a)codeweavers.com>

During the INITOUT phrase, client_do_args will go through the parameter list,
and for out parameters that are returned via pointers, it will reads those
pointers and initialize the memories they point to.

The problem is, for *_Proxy functions, the TypeFormatString generated by widl
includes an extra return value parameter, that does not have a stack location,
therefore client_do_args should not try to read it. Since the return value is
not returned via pointer, we can fix this by reordering the checks.
---
 dlls/rpcrt4/ndr_stubless.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/dlls/rpcrt4/ndr_stubless.c b/dlls/rpcrt4/ndr_stubless.c
index 8cb051443ae..ec09fefc0d7 100644
--- a/dlls/rpcrt4/ndr_stubless.c
+++ b/dlls/rpcrt4/ndr_stubless.c
@@ -508,13 +508,10 @@ void client_do_args( PMIDL_STUB_MESSAGE pStubMsg, PFORMAT_STRING pFormat, enum s
         switch (phase)
         {
         case STUBLESS_INITOUT:
-            if (*(unsigned char **)pArg)
-            {
-                if (param_needs_alloc(params[i].attr))
-                    memset( *(unsigned char **)pArg, 0, calc_arg_size( pStubMsg, pTypeFormat ));
-                else if (param_is_out_basetype(params[i].attr))
-                    memset( *(unsigned char **)pArg, 0, basetype_arg_size( params[i].u.type_format_char ));
-            }
+            if (param_needs_alloc(params[i].attr) && *(unsigned char **)pArg)
+                memset( *(unsigned char **)pArg, 0, calc_arg_size( pStubMsg, pTypeFormat ));
+            else if (param_is_out_basetype(params[i].attr) && *(unsigned char **)pArg)
+                memset( *(unsigned char **)pArg, 0, basetype_arg_size( params[i].u.type_format_char ));
             break;
         case STUBLESS_CALCSIZE:
             if (params[i].attr.IsSimpleRef && !*(unsigned char **)pArg)
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/8639

[PATCH v2 0/1] MR8639: rpcrt4: Don't read past the end of params in client_do_args. - approved

[Huw Davies (＠huw)]

This merge request was approved by Huw Davies.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8639