Skip to content

Commit 0be0db2

Browse files
authored
Add cast to ComAwareWeakReference rehydrate path (#119132)
Prevents hard to diagnose crashes caused by COM interop rehydrating wrong type.
1 parent 229aaef commit 0be0db2

File tree

3 files changed

+19
-18
lines changed

3 files changed

+19
-18
lines changed

src/libraries/System.Private.CoreLib/src/System/ComAwareWeakReference.cs

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -100,21 +100,24 @@ private void SetTarget(object? target, ComInfo? comInfo)
100100
}
101101
}
102102

103-
internal object? Target => GCHandle.InternalGet(_weakHandle) ?? RehydrateTarget();
103+
internal object? Target => GCHandle.InternalGet(_weakHandle);
104104

105-
private object? RehydrateTarget()
105+
internal nint WeakHandle => _weakHandle;
106+
107+
internal T? RehydrateTarget<T>() where T: class?
106108
{
107-
object? target = null;
109+
T? target = null;
108110
lock (this)
109111
{
110112
if (_comInfo != null)
111113
{
112-
// check if the target is still null
113-
target = GCHandle.InternalGet(_weakHandle);
114+
// Check if the target is still null
115+
target = Unsafe.As<T?>(GCHandle.InternalGet(_weakHandle));
114116
if (target == null)
115117
{
116-
// resolve and reset
117-
target = _comInfo.ResolveTarget();
118+
// Resolve and reset. Perform runtime cast to catch bugs
119+
// in COM interop where it rehydrates wrong type.
120+
target = (T?)_comInfo.ResolveTarget();
118121
if (target != null)
119122
GCHandle.InternalSet(_weakHandle, target);
120123
}
@@ -147,16 +150,10 @@ private static ComAwareWeakReference EnsureComAwareReference(ref nint taggedHand
147150
}
148151

149152
[MethodImpl(MethodImplOptions.AggressiveInlining)]
150-
internal static object? GetTarget(nint taggedHandle)
151-
{
152-
Debug.Assert((taggedHandle & ComAwareBit) != 0);
153-
return Unsafe.As<ComAwareWeakReference>(GCHandle.InternalGet(taggedHandle & ~HandleTagBits)).Target;
154-
}
155-
156-
internal static nint GetWeakHandle(nint taggedHandle)
153+
internal static ComAwareWeakReference GetFromTaggedReference(nint taggedHandle)
157154
{
158155
Debug.Assert((taggedHandle & ComAwareBit) != 0);
159-
return Unsafe.As<ComAwareWeakReference>(GCHandle.InternalGet(taggedHandle & ~HandleTagBits))._weakHandle;
156+
return Unsafe.As<ComAwareWeakReference>(GCHandle.InternalGet(taggedHandle & ~HandleTagBits));
160157
}
161158

162159
[MethodImpl(MethodImplOptions.NoInlining)]

src/libraries/System.Private.CoreLib/src/System/WeakReference.T.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,9 @@ private T? Target
141141
#if FEATURE_COMINTEROP || FEATURE_COMWRAPPERS
142142
if ((th & ComAwareBit) != 0)
143143
{
144-
target = Unsafe.As<T?>(ComAwareWeakReference.GetTarget(th));
144+
ComAwareWeakReference cwr = ComAwareWeakReference.GetFromTaggedReference(th);
145+
146+
target = Unsafe.As<T?>(cwr.Target) ?? cwr.RehydrateTarget<T>();
145147

146148
// must keep the instance alive as long as we use the handle.
147149
GC.KeepAlive(this);

src/libraries/System.Private.CoreLib/src/System/WeakReference.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ internal nint WeakHandle
107107

108108
#if FEATURE_COMINTEROP || FEATURE_COMWRAPPERS
109109
if ((th & ComAwareBit) != 0)
110-
return ComAwareWeakReference.GetWeakHandle(th);
110+
return ComAwareWeakReference.GetFromTaggedReference(th).WeakHandle;
111111
#endif
112112
return th & ~HandleTagBits;
113113
}
@@ -166,7 +166,9 @@ public virtual object? Target
166166
#if FEATURE_COMINTEROP || FEATURE_COMWRAPPERS
167167
if ((th & ComAwareBit) != 0)
168168
{
169-
target = ComAwareWeakReference.GetTarget(th);
169+
ComAwareWeakReference cwr = ComAwareWeakReference.GetFromTaggedReference(th);
170+
171+
target = cwr.Target ?? cwr.RehydrateTarget<object>();
170172

171173
// must keep the instance alive as long as we use the handle.
172174
GC.KeepAlive(this);

0 commit comments

Comments
 (0)