[PATCH 0/1] MR4112: mf/tests: Increase buffer to prevent stack corruption

[Fabian Maurer (＠DarkShadow44)]

init_dmo_media_type_video uses head + extra bytes memory,
and with MEDIASUBTYPE_RGB8 the extra bytes are already 1024.
This leads to stack corruption.

Note that not all of those are strictly necessary to prevent a crash, but I think it's safer in case it becomes relevant in the future.

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

[PATCH 1/1] mf/tests: Increase buffer to prevent stack corruption

[Fabian Maurer]

From: Fabian Maurer <dark.shadow4(a)web.de>

init_dmo_media_type_video uses head + extra bytes memory,
and with MEDIASUBTYPE_RGB8 the extra bytes are already 1024.
This leads to stack corruption.
---
 dlls/mf/tests/transform.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/dlls/mf/tests/transform.c b/dlls/mf/tests/transform.c
index c5ef1f5960a..e798601a8b5 100644
--- a/dlls/mf/tests/transform.c
+++ b/dlls/mf/tests/transform.c
@@ -1450,7 +1450,7 @@ static void check_dmo_get_output_size_info_video_(int line, IMediaObject *dmo,
 {
     DWORD size, alignment, expected_size;
     DMO_MEDIA_TYPE *type;
-    char buffer[1024];
+    char buffer[2048];
     HRESULT hr;
 
     type = (void *)buffer;
@@ -6071,7 +6071,7 @@ static void test_wmv_decoder_dmo_input_type(void)
     };
 
     DMO_MEDIA_TYPE *good_input_type, *bad_input_type, type;
-    char buffer_good_input[1024], buffer_bad_input[1024];
+    char buffer_good_input[2048], buffer_bad_input[2048];
     const GUID *input_subtype = input_subtypes[0];
     LONG width = 16, height = 16;
     VIDEOINFOHEADER *header;
@@ -6326,7 +6326,7 @@ static void test_wmv_decoder_dmo_input_type(void)
 
 static void test_wmv_decoder_dmo_output_type(void)
 {
-    char buffer_good_output[1024], buffer_bad_output[1024], buffer_input[1024];
+    char buffer_good_output[2048], buffer_bad_output[2048], buffer_input[2048];
     DMO_MEDIA_TYPE *good_output_type, *bad_output_type, *input_type, type;
     const GUID* input_subtype = &MEDIASUBTYPE_WMV1;
     LONG width = 16, height = 16;
@@ -6572,7 +6572,7 @@ static void test_wmv_decoder_media_object(void)
     IMediaObject *media_object;
     DMO_MEDIA_TYPE *type;
     const BYTE *wmv_data;
-    char buffer[1024];
+    char buffer[2048];
     HRESULT hr;
     ULONG ret;
 
-- 
GitLab

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

[Nikolay Sivov (＠nsivov)]

I think it's better to allocate on heap, if it can get that large.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4112#note_48884

[Fabian Maurer (＠DarkShadow44)]

On Mon Oct 16 22:02:01 2023 +0000, Nikolay Sivov wrote:
> I think it's better to allocate on heap, if it can get that large.
> It doesn't get bigger than `sizeof(VIDEOINFOHEADER) + 1024`, the maximum size is fix. Do we really need to put this on the heap? It wouldn't be the first time we have 2KB buffers on the stack.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4112#note_48889

[Nikolay Sivov (＠nsivov)]

On Mon Oct 16 22:02:01 2023 +0000, Fabian Maurer wrote:
> It doesn't get bigger than `sizeof(VIDEOINFOHEADER) + 1024`, the maximum
> size is fix. Do we really need to put this on the heap? It wouldn't be
> the first time we have 2KB buffers on the stack.
> Don't know, I'll leave it for @rbernon to decide.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4112#note_48891

[Rémi Bernon]

On Tue Oct 17 07:58:41 2023 +0000, Nikolay Sivov wrote:
> Don't know, I'll leave it for @rbernon to decide.
> It's more a matter of avoiding future overflows, but changing to heap allocated would require more changes and freeing every buffer so this looks simpler and good enough for now.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4112#note_48956

[PATCH 0/1] MR4112: mf/tests: Increase buffer to prevent stack corruption - approved

[Rémi Bernon]

This merge request was approved by Rémi Bernon.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4112