Kevin Puetz : oleaut32/tests: Get_test_recordinfo shouldn't point into a specific VARIANT.

[Alexandre Julliard]

Module: wine
Branch: master
Commit: 27f417eb9382e07617d37029bdca3bbaadf9f10d
URL:    https://gitlab.winehq.org/wine/wine/-/commit/27f417eb9382e07617d37029bdca3bb...

Author: Kevin Puetz <PuetzKevinA(a)JohnDeere.com>
Date:   Thu Sep 22 23:50:29 2022 -0500

oleaut32/tests: Get_test_recordinfo shouldn't point into a specific VARIANT.

In many cases (in particular when VariantCopy has been used),
multiple VARIANT structs which contain the same type of record
will share the same instance of IRecordInfo, just adding refcounts.

RecordClear should not be aware of where the data it's clearing came from,
and certainly should not be modifying someone else's VARIANT::brecVal.
This seems to have been intended as a way to make tests more sensitive to
use-after-free, by overwriting the source pointer with NULL after
clearing the pointed-to record.

To preserve that sensitivity (if it was indeed the goal), replace the
hardcoded "0xdeadbeef is valid" check with a "validsrc" address that
get_test_recordinfo initially agrees to pretend points to a valid record,
but will stop accepting after a call to RecordClear.

---

 dlls/oleaut32/tests/vartest.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/dlls/oleaut32/tests/vartest.c b/dlls/oleaut32/tests/vartest.c
index cdbb836b041..342133d6403 100644
--- a/dlls/oleaut32/tests/vartest.c
+++ b/dlls/oleaut32/tests/vartest.c
@@ -246,10 +246,10 @@ typedef struct IRecordInfoImpl
 {
     IRecordInfo IRecordInfo_iface;
     LONG ref;
+    void *validsrc;
     unsigned int recordclear;
     unsigned int getsize;
     unsigned int recordcopy;
-    struct __tagBRECORD *rec;
 } IRecordInfoImpl;
 
 static inline IRecordInfoImpl *impl_from_IRecordInfo(IRecordInfo *iface)
@@ -299,7 +299,8 @@ static HRESULT WINAPI RecordInfo_RecordClear(IRecordInfo *iface, void *data)
 {
     IRecordInfoImpl* This = impl_from_IRecordInfo(iface);
     This->recordclear++;
-    This->rec->pvRecord = NULL;
+    if(data == This->validsrc)
+        This->validsrc = NULL; /* not valid anymore, now that it's been cleared */
     return S_OK;
 }
 
@@ -307,7 +308,7 @@ static HRESULT WINAPI RecordInfo_RecordCopy(IRecordInfo *iface, void *src, void
 {
     IRecordInfoImpl* This = impl_from_IRecordInfo(iface);
     This->recordcopy++;
-    ok(src == (void*)0xdeadbeef, "wrong src pointer %p\n", src);
+    ok(This->validsrc && (src == This->validsrc), "wrong src pointer %p\n", src);
     return S_OK;
 }
 
@@ -430,6 +431,7 @@ static IRecordInfoImpl *get_test_recordinfo(void)
     rec->recordclear = 0;
     rec->getsize = 0;
     rec->recordcopy = 0;
+    rec->validsrc = (void *)0xdeadbeef;
 
     return rec;
 }
@@ -767,7 +769,6 @@ static test_VariantClearImpl test_myVariantClearImpl = {{&test_VariantClear_vtbl
 
 static void test_VariantClear(void)
 {
-  struct __tagBRECORD *rec;
   IRecordInfoImpl *recinfo;
   HRESULT hres;
   VARIANTARG v;
@@ -892,15 +893,12 @@ static void test_VariantClear(void)
   /* RECORD */
   recinfo = get_test_recordinfo();
   V_VT(&v) = VT_RECORD;
-  rec = &V_UNION(&v, brecVal);
-  rec->pRecInfo = &recinfo->IRecordInfo_iface;
-  rec->pvRecord = (void*)0xdeadbeef;
+  V_RECORDINFO(&v) = &recinfo->IRecordInfo_iface;
+  V_RECORD(&v) = recinfo->validsrc;
   recinfo->recordclear = 0;
   recinfo->ref = 2;
-  recinfo->rec = rec;
   hres = VariantClear(&v);
   ok(hres == S_OK, "ret %08lx\n", hres);
-  ok(rec->pvRecord == NULL, "got %p\n", rec->pvRecord);
   ok(recinfo->recordclear == 1, "got %d\n", recinfo->recordclear);
   ok(recinfo->ref == 1, "got %ld\n", recinfo->ref);
   IRecordInfo_Release(&recinfo->IRecordInfo_iface);
@@ -908,7 +906,6 @@ static void test_VariantClear(void)
 
 static void test_VariantCopy(void)
 {
-  struct __tagBRECORD *rec;
   IRecordInfoImpl *recinfo;
   VARIANTARG vSrc, vDst;
   VARTYPE vt;
@@ -1033,20 +1030,17 @@ static void test_VariantCopy(void)
   V_VT(&vDst) = VT_EMPTY;
 
   V_VT(&vSrc) = VT_RECORD;
-  rec = &V_UNION(&vSrc, brecVal);
-  rec->pRecInfo = &recinfo->IRecordInfo_iface;
-  rec->pvRecord = (void*)0xdeadbeef;
+  V_RECORDINFO(&vSrc) = &recinfo->IRecordInfo_iface;
+  V_RECORD(&vSrc) = recinfo->validsrc;
 
   recinfo->recordclear = 0;
   recinfo->recordcopy = 0;
   recinfo->getsize = 0;
-  recinfo->rec = rec;
   hres = VariantCopy(&vDst, &vSrc);
   ok(hres == S_OK, "ret %08lx\n", hres);
 
-  rec = &V_UNION(&vDst, brecVal);
-  ok(rec->pvRecord != (void*)0xdeadbeef && rec->pvRecord != NULL, "got %p\n", rec->pvRecord);
-  ok(rec->pRecInfo == &recinfo->IRecordInfo_iface, "got %p\n", rec->pRecInfo);
+  ok(V_RECORD(&vDst) != recinfo->validsrc && V_RECORD(&vDst) != NULL, "got %p\n", V_RECORD(&vDst));
+  ok(V_RECORDINFO(&vDst) == &recinfo->IRecordInfo_iface, "got %p\n", V_RECORDINFO(&vDst));
   ok(recinfo->getsize == 1, "got %d\n", recinfo->recordclear);
   ok(recinfo->recordcopy == 1, "got %d\n", recinfo->recordclear);