Skip to content

Commit b35a3cc

Browse files
committed
Edge case: prevent in parameter mutations
... by creating defensive copies of byref-like `in` arguments and ref- erencing those copies instead of the actual parameters.
1 parent d3eb112 commit b35a3cc

1 file changed

Lines changed: 29 additions & 5 deletions

File tree

src/Castle.Core/DynamicProxy/Generators/MethodWithInvocationGenerator.cs

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ protected override MethodEmitter BuildProxiedMethodBody(MethodEmitter emitter, C
104104

105105
var argumentsMarshaller = new ArgumentsMarshaller(emitter, MethodToOverride.GetParameters());
106106

107-
argumentsMarshaller.CopyIn(out var argumentsArray, out var hasByRefArguments, out var hasByRefLikeArguments);
107+
argumentsMarshaller.CopyIn(out var argumentsArray, out var hasByRefArguments, out var hasByRefLikeArguments, out var byRefLikeBuffers);
108108

109109
var ctorArguments = GetCtorArguments(@class, proxiedMethodTokenExpression, argumentsArray, methodInterceptors);
110110
ctorArguments = ModifyArguments(@class, ctorArguments);
@@ -141,7 +141,7 @@ protected override MethodEmitter BuildProxiedMethodBody(MethodEmitter emitter, C
141141

142142
if (hasByRefLikeArguments)
143143
{
144-
argumentsMarshaller.InvalidateByRefLikeProxies(argumentsArray);
144+
argumentsMarshaller.InvalidateByRefLikeProxies(argumentsArray, byRefLikeBuffers);
145145
}
146146

147147
if (returnValueBuffer != null)
@@ -254,13 +254,14 @@ public ArgumentsMarshaller(MethodEmitter method, ParameterInfo[] parameters)
254254
this.parameters = parameters;
255255
}
256256

257-
public void CopyIn(out LocalReference argumentsArray, out bool hasByRefArguments, out bool hasByRefLikeArguments)
257+
public void CopyIn(out LocalReference argumentsArray, out bool hasByRefArguments, out bool hasByRefLikeArguments, out LocalReference[] byRefLikeBuffers)
258258
{
259259
var arguments = method.Arguments;
260260

261261
argumentsArray = method.CodeBuilder.DeclareLocal(typeof(object[]));
262262
hasByRefArguments = false;
263263
hasByRefLikeArguments = false;
264+
byRefLikeBuffers = null;
264265

265266
method.CodeBuilder.AddStatement(
266267
new AssignStatement(
@@ -287,6 +288,18 @@ public void CopyIn(out LocalReference argumentsArray, out bool hasByRefArguments
287288
{
288289
hasByRefLikeArguments = true;
289290

291+
if (argument.Type.IsByRef && parameters[i].IsReadOnly)
292+
{
293+
// For by-reference `in` parameters, we create a defensive local copy of the argument
294+
// so that (erroneous) mutations to it performed by any interceptor can't propagate back
295+
// to the caller:
296+
var buffer = method.CodeBuilder.DeclareLocal(dereferencedArgumentType);
297+
method.CodeBuilder.AddStatement(new AssignStatement(buffer, dereferencedArgument));
298+
byRefLikeBuffers ??= new LocalReference[n];
299+
byRefLikeBuffers[i] = buffer;
300+
dereferencedArgument = buffer;
301+
}
302+
290303
// Byref-like values live exclusively on the stack and cannot be boxed to `object`.
291304
// Instead of them, we prepare instances of `ByRefLikeReference` wrappers that reference them.
292305
var referenceCtor = GetByRefLikeReferenceCtorFor(dereferencedArgumentType);
@@ -336,14 +349,25 @@ public void CopyOut(LocalReference argumentsArray)
336349
}
337350
}
338351

339-
public void InvalidateByRefLikeProxies(LocalReference argumentsArray)
352+
public void InvalidateByRefLikeProxies(LocalReference argumentsArray, LocalReference[] byRefLikeBuffers)
340353
{
341354
#if FEATURE_BYREFLIKE
342355
var arguments = method.Arguments;
343356

344357
for (int i = 0, n = arguments.Length; i < n; ++i)
345358
{
346-
var argument = arguments[i];
359+
Reference argument = arguments[i];
360+
361+
if (byRefLikeBuffers?[i] != null)
362+
{
363+
Debug.Assert(parameters[i].IsByRef && parameters[i].IsReadOnly);
364+
365+
// We previously created a defensive local copy for this `in` parameter
366+
// to prevent any mutations escaping to the caller. Redirect `argument`
367+
// to that local copy so the below invalidation won't fail its address check:
368+
argument = byRefLikeBuffers[i];
369+
}
370+
347371
var argumentType = argument.Type;
348372
var dereferencedArgumentType = argumentType.IsByRef ? argumentType.GetElementType()! : argumentType;
349373

0 commit comments

Comments
 (0)