[PATCH 0/2] MR8297: msvcirt/tests: Fix 2 memory issues in tests

[PATCH 1/2] msvcirt/tests: Fix use-after-free in test_ifstream.

[Yuxuan Shui]

From: Yuxuan Shui

Instead of logging value from the current buffer, we used a saved buffer
pointer which has already been freed.
---
 dlls/msvcirt/tests/msvcirt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/msvcirt/tests/msvcirt.c b/dlls/msvcirt/tests/msvcirt.c
index a22984a3e86..8b20c6095e3 100644
--- a/dlls/msvcirt/tests/msvcirt.c
+++ b/dlls/msvcirt/tests/msvcirt.c
@@ -7380,7 +7380,7 @@ static void test_ifstream(void)
     call_func5(p_ifstream_buffer_ctor, &ifs, -1, NULL, 0, TRUE);
     ok(ifs.base_ios.sb->base == NULL, "wrong base value, expected NULL got %p\n", ifs.base_ios.sb->base);
     ok(ifs.base_ios.sb->ebuf == NULL, "wrong ebuf value, expected NULL got %p\n", ifs.base_ios.sb->ebuf);
-    ok(ifs.base_ios.sb->unbuffered == 1, "wrong unbuffered value, expected 1 got %d\n", pfb->base.unbuffered);
+    ok(ifs.base_ios.sb->unbuffered == 1, "wrong unbuffered value, expected 1 got %d\n", ifs.base_ios.sb->unbuffered);
     ok(ifs.base_ios.sb->allocated == 0, "wrong allocated value, expected 0 got %d\n", ifs.base_ios.sb->allocated);

psb = call_func3(p_ifstream_setbuf, &ifs, buffer, ARRAY_SIZE(buffer));

[PATCH 2/2] msvcirt/tests: Avoid out-of-bound access in test_strstreambuf.

[Yuxuan Shui]

From: Yuxuan Shui

---
 dlls/msvcirt/tests/msvcirt.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/dlls/msvcirt/tests/msvcirt.c b/dlls/msvcirt/tests/msvcirt.c
index 8b20c6095e3..bf2b6381608 100644
--- a/dlls/msvcirt/tests/msvcirt.c
+++ b/dlls/msvcirt/tests/msvcirt.c
@@ -2265,6 +2265,15 @@ static void test_filebuf(void)
     CloseHandle(thread);
 }

+static void *custom_alloc(LONG size)
+{
+    /* this allocates size + 16 bytes. a test later validates that strstreambuf_overflow
+     * always allocates in `strstreambuf.increase` increments, even when the resultant buffer
+     * won't be large enough to contain `pptr`. to avoid actually writing out-of-bound, we
+     * allocate a few bytes more here. */
+    return p_operator_new(size + 16);
+}
+
 static void test_strstreambuf(void)
 {
     strstreambuf ssb1, ssb2;
@@ -2532,8 +2541,12 @@ static void test_strstreambuf(void)
     ok(ssb2.base.pptr == ssb2.base.base + 1, "wrong put pointer, expected %p got %p\n", ssb2.base.base + 1, ssb2.base.pptr);
     ok(ssb2.base.epptr == ssb2.base.base + 16, "wrong put end, expected %p got %p\n", ssb2.base.base + 16, ssb2.base.epptr);
     ok(*(ssb2.base.pptr - 1) == 'D', "expected 'D' got %d\n", *(ssb2.base.pptr - 1));
+    /* test strstreambuf_overflow always allocate in `increase` increments, even when the new size
+     * won't be large enough to contain `pptr`. `custom_alloc` allocate more space than asked for
+     * to avoid out-of-bound access. */
     ssb2.base.pbase = ssb2.base.base + 3;
     ssb2.base.pptr = ssb2.base.epptr + 5;
+    ssb2.f_alloc = custom_alloc;
     ret = (int) call_func2(p_strstreambuf_overflow, &ssb2, 'E');
     ok(ret == 1, "expected 1 got %d\n", ret);
     ok(ssb2.base.ebuf == ssb2.base.base + 20, "expected %p got %p\n", ssb2.base.base + 20, ssb2.base.ebuf);
@@ -2542,6 +2555,7 @@ static void test_strstreambuf(void)
     ok(ssb2.base.pptr == ssb2.base.base + 22, "wrong put pointer, expected %p got %p\n", ssb2.base.base + 22, ssb2.base.pptr);
     ok(ssb2.base.epptr == ssb2.base.base + 20, "wrong put end, expected %p got %p\n", ssb2.base.base + 20, ssb2.base.epptr);
     ok(*(ssb2.base.pptr - 1) == 'E', "expected 'E' got %d\n", *(ssb2.base.pptr - 1));
+    ssb2.f_alloc = NULL;
     ssb2.base.egptr = ssb2.base.base + 2;
     ret = (int) call_func2(p_strstreambuf_overflow, &ssb2, 'F');
     ok(ret == 1, "expected 1 got %d\n", ret);

[Piotr Caban (＠piotr)]

Piotr Caban (@piotr) commented about dlls/msvcirt/tests/msvcirt.c:
>  ok(ssb2.base.pptr == ssb2.base.base + 1, "wrong put pointer, expected %p got %p\n", ssb2.base.base + 1, ssb2.base.pptr);
>  ok(ssb2.base.epptr == ssb2.base.base + 16, "wrong put end, expected %p got %p\n", ssb2.base.base + 16, ssb2.base.epptr);
>  ok(*(ssb2.base.pptr - 1) == 'D', "expected 'D' got %d\n", *(ssb2.base.pptr - 1));
> - /* test strstreambuf_overflow always allocate in `increase` increments, even when the new size
> -
> * won't be large enough to contain `pptr`. `custom_alloc` allocate more space than asked for
> -
> * to avoid out-of-bound access. */
> ssb2.base.pbase = ssb2.base.base + 3;
>    ssb2.base.pptr = ssb2.base.epptr + 5;
> - ssb2.f_alloc = custom_alloc;
> ret = (int) call_func2(p_strstreambuf_overflow, &ssb2, 'E');

Can we change the test to call strstreambuf_doallocate instead? I think it would be cleaner this way.

[Yuxuan Shui (＠yshui)]

On Fri Jun 13 15:35:53 2025 +0000, Piotr Caban wrote:
> Can we change the test to call strstreambuf_doallocate instead? I think
> it would be cleaner this way.

Did you mean call `strstreambuf_doallocate` directly instead of `strstreambuf_overflow`? We can do that but I feel if we do that we can't rule out the possibility that `strstreambuf_overflow` may call `doallocate` multiple times. (i.e. `doallocate` does one `increase` increment, but `overflow` does multiple).

[Piotr Caban (＠piotr)]

On Sat Jun 14 01:35:28 2025 +0000, Yuxuan Shui wrote:
> Did you mean call `strstreambuf_doallocate` directly instead of
> `strstreambuf_overflow`? We can do that but I feel if we do that we
> can't rule out the possibility that `strstreambuf_overflow` may call
> `doallocate` multiple times. (i.e. `doallocate` does one `increase`
> increment, but `overflow` does multiple).

I was not sure if there's value in testing that.It's possible to set the pointers using exported functions so the test shows limitation of native implementation. I think I'm good with current approach.

[PATCH 0/2] MR8297: msvcirt/tests: Fix 2 memory issues in tests - approved

[Piotr Caban (＠piotr)]

This merge request was approved by Piotr Caban.