[PATCH 0/1] MR1130: wbemprox: Fix string length in get_value_bstr().

[Paul Gofman (＠gofman)]

Currently the resulting string is missing closing quote.

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

[PATCH 1/1] wbemprox: Fix string length in get_value_bstr().

[Paul Gofman]

From: Paul Gofman <pgofman(a)codeweavers.com>

---
 dlls/wbemprox/table.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/wbemprox/table.c b/dlls/wbemprox/table.c
index 34b09bc872f..4ce198c0ab7 100644
--- a/dlls/wbemprox/table.c
+++ b/dlls/wbemprox/table.c
@@ -176,7 +176,7 @@ BSTR get_value_bstr( const struct table *table, UINT row, UINT column )
     case CIM_REFERENCE:
     case CIM_STRING:
         if (!val) return NULL;
-        len = lstrlenW( (const WCHAR *)(INT_PTR)val ) + 2;
+        len = lstrlenW( (const WCHAR *)(INT_PTR)val ) + 3;
         if (!(ret = SysAllocStringLen( NULL, len ))) return NULL;
         swprintf( ret, len, L"\"%s\"", (const WCHAR *)(INT_PTR)val );
         return ret;
-- 
GitLab

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

[PATCH 1/1] wbemprox: Fix string length in get_value_bstr().

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=125220

Your paranoid android.


=== debian11 (build log) ===

Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24741.
Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24741.
Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24741.

[Nikolay Sivov (＠nsivov)]

Is it possible to test this? It seems unnecessary, because SysAllocStringLen() should allocate one WCHAR more for null, so you get string length + 2 quote marks.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1130#note_11641

[PATCH 0/1] MR1130: wbemprox: Fix string length in get_value_bstr(). - approved

[Hans Leidekker (＠hans)]

This merge request was approved by Hans Leidekker.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1130

[Hans Leidekker (＠hans)]

Yes, but the length is also passed to swprintf().

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1130#note_11670

[Paul Gofman (＠gofman)]

On Fri Oct 21 10:15:42 2022 +0000, Hans Leidekker wrote:
> Yes, but the length is also passed to swprintf().
> Yes, there wasn’t a past end write, it is swprintf was visibly swallowing the closing quote.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1130#note_11688

[Esme Povirk (＠madewokherd)]

On Fri Oct 21 10:15:42 2022 +0000, Paul Gofman wrote:
> Yes, there wasn’t a past end write, it is swprintf was visibly
> swallowing the closing quote.
> It seems like this would make the length in the BSTR too long by including the terminating null? It's hard to tell whether this actually matters, but it seems like the BSTR can be returned to applications via class_object_Get->get_propval->get_value_bstr.

The BSTR will be allocated with space for the len characters plus a null terminator. If swprintf expects a length including null terminator, I think it would be OK to pass in len+1 to swprintf.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1130#note_11751