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.

oleaut32/tests: Get_test_recordinfo shouldn't point into a specific VARIANT.
27f417eb · Kevin Puetz · Alexandre Julliard · 3265d7b5 · 27f417eb
Commit 27f417eb authored Sep 22, 2022 by Kevin Puetz Committed by Alexandre Julliard Sep 26, 2022
Hide whitespace changes
Inline Side-by-side

Showing with 11 additions and 17 deletions

vartest.c dlls/oleaut32/tests/vartest.c +11 -17

No files found.
--- 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);