Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a potential out-of-bound read in RuntimeInvokeHostAssemblyResolver #104536

Merged
merged 7 commits into from
Jul 28, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -598,12 +598,12 @@ private CultureInfo GetLocale()
[LibraryImport(RuntimeHelpers.QCall, EntryPoint = "AssemblyNative_GetSimpleName")]
private static partial void GetSimpleName(QCallAssembly assembly, StringHandleOnStack retSimpleName);

internal string? GetSimpleName()
internal string GetSimpleName()
{
RuntimeAssembly runtimeAssembly = this;
string? name = null;
GetSimpleName(new QCallAssembly(ref runtimeAssembly), new StringHandleOnStack(ref name));
return name;
return name!;
}

[LibraryImport(RuntimeHelpers.QCall, EntryPoint = "AssemblyNative_GetHashAlgorithm")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ internal Assembly LoadFromInMemoryModule(IntPtr moduleHandle)

// This method is invoked by the VM to resolve a satellite assembly reference
// after trying assembly resolution via Load override without success.
private static Assembly? ResolveSatelliteAssembly(IntPtr gchManagedAssemblyLoadContext, AssemblyName assemblyName)
private static RuntimeAssembly? ResolveSatelliteAssembly(IntPtr gchManagedAssemblyLoadContext, AssemblyName assemblyName)
{
AssemblyLoadContext context = (AssemblyLoadContext)(GCHandle.FromIntPtr(gchManagedAssemblyLoadContext).Target)!;

Expand All @@ -136,7 +136,7 @@ private static IntPtr ResolveUnmanagedDllUsingEvent(string unmanagedDllName, Ass

// This method is invoked by the VM to resolve an assembly reference using the Resolving event
// after trying assembly resolution via Load override and TPA load context without success.
private static Assembly? ResolveUsingResolvingEvent(IntPtr gchManagedAssemblyLoadContext, AssemblyName assemblyName)
private static RuntimeAssembly? ResolveUsingResolvingEvent(IntPtr gchManagedAssemblyLoadContext, AssemblyName assemblyName)
{
AssemblyLoadContext context = (AssemblyLoadContext)(GCHandle.FromIntPtr(gchManagedAssemblyLoadContext).Target)!;
// Invoke the AssemblyResolve event callbacks if wired up
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/dlls/mscorrc/mscorrc.rc
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,7 @@ END
STRINGTABLE DISCARDABLE
BEGIN
IDS_HOST_ASSEMBLY_RESOLVER_ASSEMBLY_ALREADY_LOADED_IN_CONTEXT "Assembly with same name is already loaded"
IDS_HOST_ASSEMBLY_RESOLVER_DYNAMICALLY_EMITTED_ASSEMBLIES_UNSUPPORTED "Dynamically emitted assemblies are unsupported during host-based resolution."
IDS_HOST_ASSEMBLY_RESOLVER_DYNAMICALLY_EMITTED_ASSEMBLIES_UNSUPPORTED "Dynamically emitted assemblies are unsupported for AssemblyLoadContext resolution."
END

STRINGTABLE DISCARDABLE
Expand Down
40 changes: 24 additions & 16 deletions src/coreclr/vm/appdomain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3965,7 +3965,7 @@ Assembly* AppDomain::RaiseTypeResolveEventThrowing(Assembly* pAssembly, LPCSTR s
GCX_COOP();

struct {
OBJECTREF AssemblyRef;
ASSEMBLYREF AssemblyRef;
STRINGREF str;
} gc;
gc.AssemblyRef = NULL;
Expand All @@ -3974,7 +3974,7 @@ Assembly* AppDomain::RaiseTypeResolveEventThrowing(Assembly* pAssembly, LPCSTR s
GCPROTECT_BEGIN(gc);

if (pAssembly != NULL)
gc.AssemblyRef = pAssembly->GetExposedObject();
gc.AssemblyRef = (ASSEMBLYREF)pAssembly->GetExposedObject();

MethodDescCallSite onTypeResolve(METHOD__ASSEMBLYLOADCONTEXT__ON_TYPE_RESOLVE);

Expand All @@ -3984,14 +3984,16 @@ Assembly* AppDomain::RaiseTypeResolveEventThrowing(Assembly* pAssembly, LPCSTR s
ObjToArgSlot(gc.AssemblyRef),
ObjToArgSlot(gc.str)
};
ASSEMBLYREF ResultingAssemblyRef = (ASSEMBLYREF) onTypeResolve.Call_RetOBJECTREF(args);
gc.AssemblyRef = (ASSEMBLYREF) onTypeResolve.Call_RetOBJECTREF(args);

if (ResultingAssemblyRef != NULL)
if (gc.AssemblyRef != NULL)
{
pResolvedAssembly = ResultingAssemblyRef->GetAssembly();
_ASSERTE(CoreLibBinder::IsClass(gc.AssemblyRef->GetMethodTable(), CLASS__ASSEMBLY));

pResolvedAssembly = gc.AssemblyRef->GetAssembly();

if (pResultingAssemblyRef)
*pResultingAssemblyRef = ResultingAssemblyRef;
*pResultingAssemblyRef = gc.AssemblyRef;
else
{
if (pResolvedAssembly->IsCollectible())
Expand Down Expand Up @@ -4023,7 +4025,7 @@ Assembly* AppDomain::RaiseResourceResolveEvent(Assembly* pAssembly, LPCSTR szNam
GCX_COOP();

struct {
OBJECTREF AssemblyRef;
ASSEMBLYREF AssemblyRef;
STRINGREF str;
} gc;
gc.AssemblyRef = NULL;
Expand All @@ -4032,7 +4034,7 @@ Assembly* AppDomain::RaiseResourceResolveEvent(Assembly* pAssembly, LPCSTR szNam
GCPROTECT_BEGIN(gc);

if (pAssembly != NULL)
gc.AssemblyRef=pAssembly->GetExposedObject();
gc.AssemblyRef=(ASSEMBLYREF)pAssembly->GetExposedObject();

MethodDescCallSite onResourceResolve(METHOD__ASSEMBLYLOADCONTEXT__ON_RESOURCE_RESOLVE);
gc.str = StringObject::NewString(szName);
Expand All @@ -4041,10 +4043,12 @@ Assembly* AppDomain::RaiseResourceResolveEvent(Assembly* pAssembly, LPCSTR szNam
ObjToArgSlot(gc.AssemblyRef),
ObjToArgSlot(gc.str)
};
ASSEMBLYREF ResultingAssemblyRef = (ASSEMBLYREF) onResourceResolve.Call_RetOBJECTREF(args);
if (ResultingAssemblyRef != NULL)
gc.AssemblyRef = (ASSEMBLYREF) onResourceResolve.Call_RetOBJECTREF(args);
if (gc.AssemblyRef != NULL)
{
pResolvedAssembly = ResultingAssemblyRef->GetAssembly();
_ASSERTE(CoreLibBinder::IsClass(gc.AssemblyRef->GetMethodTable(), CLASS__ASSEMBLY));

pResolvedAssembly = gc.AssemblyRef->GetAssembly();
if (pResolvedAssembly->IsCollectible())
{
COMPlusThrow(kNotSupportedException, W("NotSupported_CollectibleAssemblyResolve"));
Expand Down Expand Up @@ -4085,7 +4089,7 @@ AppDomain::RaiseAssemblyResolveEvent(
Assembly* pAssembly = NULL;

struct {
OBJECTREF AssemblyRef;
ASSEMBLYREF AssemblyRef;
STRINGREF str;
} gc;
gc.AssemblyRef = NULL;
Expand All @@ -4095,7 +4099,7 @@ AppDomain::RaiseAssemblyResolveEvent(
{
if (pSpec->GetParentAssembly() != NULL)
{
gc.AssemblyRef=pSpec->GetParentAssembly()->GetExposedObject();
gc.AssemblyRef=(ASSEMBLYREF)pSpec->GetParentAssembly()->GetExposedObject();
}

MethodDescCallSite onAssemblyResolve(METHOD__ASSEMBLYLOADCONTEXT__ON_ASSEMBLY_RESOLVE);
Expand All @@ -4106,11 +4110,13 @@ AppDomain::RaiseAssemblyResolveEvent(
ObjToArgSlot(gc.str)
};

ASSEMBLYREF ResultingAssemblyRef = (ASSEMBLYREF) onAssemblyResolve.Call_RetOBJECTREF(args);
gc.AssemblyRef = (ASSEMBLYREF) onAssemblyResolve.Call_RetOBJECTREF(args);

if (ResultingAssemblyRef != NULL)
if (gc.AssemblyRef != NULL)
{
pAssembly = ResultingAssemblyRef->GetAssembly();
_ASSERTE(CoreLibBinder::IsClass(gc.AssemblyRef->GetMethodTable(), CLASS__ASSEMBLY));

pAssembly = gc.AssemblyRef->GetAssembly();
if (pAssembly->IsCollectible())
{
COMPlusThrow(kNotSupportedException, W("NotSupported_CollectibleAssemblyResolve"));
Expand Down Expand Up @@ -4543,6 +4549,8 @@ HRESULT RuntimeInvokeHostAssemblyResolver(INT_PTR pManagedAssemblyLoadContextToB
// If we are here, assembly was successfully resolved via Load or Resolving events.
_ASSERTE(_gcRefs.oRefLoadedAssembly != NULL);

_ASSERTE(CoreLibBinder::IsClass(_gcRefs.oRefLoadedAssembly->GetMethodTable(), CLASS__ASSEMBLY));

// We were able to get the assembly loaded. Now, get its name since the host could have
// performed the resolution using an assembly with different name.
DomainAssembly *pDomainAssembly = _gcRefs.oRefLoadedAssembly->GetDomainAssembly();
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/vm/corelib.h
Original file line number Diff line number Diff line change
Expand Up @@ -867,11 +867,11 @@ DEFINE_FIELD_U(_id, AssemblyLoadContextBaseObject, _id)
DEFINE_FIELD_U(_state, AssemblyLoadContextBaseObject, _state)
DEFINE_FIELD_U(_isCollectible, AssemblyLoadContextBaseObject, _isCollectible)
DEFINE_CLASS(ASSEMBLYLOADCONTEXT, Loader, AssemblyLoadContext)
DEFINE_METHOD(ASSEMBLYLOADCONTEXT, RESOLVE, Resolve, SM_IntPtr_AssemblyName_RetAssemblyBase)
DEFINE_METHOD(ASSEMBLYLOADCONTEXT, RESOLVE, Resolve, SM_IntPtr_AssemblyName_RetAssembly)
DEFINE_METHOD(ASSEMBLYLOADCONTEXT, RESOLVEUNMANAGEDDLL, ResolveUnmanagedDll, SM_Str_IntPtr_RetIntPtr)
DEFINE_METHOD(ASSEMBLYLOADCONTEXT, RESOLVEUNMANAGEDDLLUSINGEVENT, ResolveUnmanagedDllUsingEvent, SM_Str_AssemblyBase_IntPtr_RetIntPtr)
DEFINE_METHOD(ASSEMBLYLOADCONTEXT, RESOLVEUSINGEVENT, ResolveUsingResolvingEvent, SM_IntPtr_AssemblyName_RetAssemblyBase)
DEFINE_METHOD(ASSEMBLYLOADCONTEXT, RESOLVESATELLITEASSEMBLY, ResolveSatelliteAssembly, SM_IntPtr_AssemblyName_RetAssemblyBase)
DEFINE_METHOD(ASSEMBLYLOADCONTEXT, RESOLVEUSINGEVENT, ResolveUsingResolvingEvent, SM_IntPtr_AssemblyName_RetAssembly)
DEFINE_METHOD(ASSEMBLYLOADCONTEXT, RESOLVESATELLITEASSEMBLY, ResolveSatelliteAssembly, SM_IntPtr_AssemblyName_RetAssembly)
DEFINE_FIELD(ASSEMBLYLOADCONTEXT, ASSEMBLY_LOAD, AssemblyLoad)
DEFINE_METHOD(ASSEMBLYLOADCONTEXT, ON_ASSEMBLY_LOAD, OnAssemblyLoad, SM_Assembly_RetVoid)
DEFINE_METHOD(ASSEMBLYLOADCONTEXT, ON_RESOURCE_RESOLVE, OnResourceResolve, SM_Assembly_Str_RetAssembly)
Expand Down
6 changes: 1 addition & 5 deletions src/coreclr/vm/metasig.h
Original file line number Diff line number Diff line change
Expand Up @@ -393,8 +393,6 @@ DEFINE_METASIG(IM(IntPtr_Int_RetVoid, I i, v))
DEFINE_METASIG(IM(IntInt_RetArrByte, i i, a(b)))
DEFINE_METASIG(IM(RetIntPtr, _, I))
DEFINE_METASIG(IM(RetInt, _, i))
DEFINE_METASIG_T(IM(RetAssemblyName, _, C(ASSEMBLY_NAME)))
DEFINE_METASIG_T(IM(RetAssemblyBase, _, C(ASSEMBLYBASE)))
DEFINE_METASIG_T(IM(RetModule, _, C(MODULE)))
DEFINE_METASIG_T(IM(PtrNativeAssemblyNameParts, P(g(NATIVE_ASSEMBLY_NAME_PARTS)), v))
DEFINE_METASIG(SM(PtrCharPtrVoid, P(u) P(v), v))
Expand Down Expand Up @@ -456,8 +454,6 @@ DEFINE_METASIG(IM(Int_Int_Int_Int_RetVoid, i i i i, v))
DEFINE_METASIG_T(IM(Obj_EventArgs_RetVoid, j C(EVENT_ARGS), v))
DEFINE_METASIG_T(IM(Obj_UnhandledExceptionEventArgs_RetVoid, j C(UNHANDLED_EVENTARGS), v))

DEFINE_METASIG_T(IM(Assembly_RetBool, C(ASSEMBLY), F))
DEFINE_METASIG_T(IM(AssemblyBase_RetBool, C(ASSEMBLYBASE), F))
DEFINE_METASIG_T(IM(Exception_RetVoid, C(EXCEPTION), v))

DEFINE_METASIG(IM(IntPtr_RetObj, I, j))
Expand Down Expand Up @@ -565,7 +561,7 @@ DEFINE_METASIG_T(IM(RefGuid_OutIntPtr_RetCustomQueryInterfaceResult, r(g(GUID))
DEFINE_METASIG_T(SM(RefGuid_RefGuid_RetVoid, r(g(GUID)) r(g(GUID)) , v))
DEFINE_METASIG_T(SM(RefGuid_RetVoid, r(g(GUID)), v))

DEFINE_METASIG_T(SM(IntPtr_AssemblyName_RetAssemblyBase, I C(ASSEMBLY_NAME), C(ASSEMBLYBASE)))
DEFINE_METASIG_T(SM(IntPtr_AssemblyName_RetAssembly, I C(ASSEMBLY_NAME), C(ASSEMBLY)))
DEFINE_METASIG_T(SM(Str_AssemblyBase_IntPtr_RetIntPtr, s C(ASSEMBLYBASE) I, I))
DEFINE_METASIG_T(SM(Str_AssemblyBase_Bool_UInt_RetIntPtr, s C(ASSEMBLYBASE) F K, I))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@ public void Dispose()
#if !NATIVEAOT
// This method is invoked by the VM when using the host-provided assembly load context
// implementation.
private static Assembly? Resolve(IntPtr gchManagedAssemblyLoadContext, AssemblyName assemblyName)
private static RuntimeAssembly? Resolve(IntPtr gchManagedAssemblyLoadContext, AssemblyName assemblyName)
jkotas marked this conversation as resolved.
Show resolved Hide resolved
{
AssemblyLoadContext context = (AssemblyLoadContext)(GCHandle.FromIntPtr(gchManagedAssemblyLoadContext).Target)!;

Expand Down Expand Up @@ -644,56 +644,44 @@ public void Dispose()
return null;
}

private static Assembly ValidateAssemblyNameWithSimpleName(Assembly assembly, string? requestedSimpleName)
private static RuntimeAssembly ValidateAssemblyNameWithSimpleName(Assembly assembly, string? requestedSimpleName)
{
ArgumentException.ThrowIfNullOrEmpty(requestedSimpleName, "AssemblyName.Name");

// Get the name of the loaded assembly
string? loadedSimpleName = null;

// Derived type's Load implementation is expected to use one of the LoadFrom* methods to get the assembly
// which is a RuntimeAssembly instance. However, since Assembly type can be used build any other artifact (e.g. AssemblyBuilder),
// we need to check for RuntimeAssembly.
RuntimeAssembly? rtLoadedAssembly = GetRuntimeAssembly(assembly);
if (rtLoadedAssembly != null)
RuntimeAssembly? runtimeAssembly = GetRuntimeAssembly(assembly);
if (runtimeAssembly == null)
{
loadedSimpleName = rtLoadedAssembly.GetSimpleName();
throw new InvalidOperationException(SR.Argument_MustBeRuntimeAssembly);
jkotas marked this conversation as resolved.
Show resolved Hide resolved
}

// The simple names should match at the very least
if (string.IsNullOrEmpty(loadedSimpleName) || !requestedSimpleName.Equals(loadedSimpleName, StringComparison.InvariantCultureIgnoreCase))
if (!requestedSimpleName.Equals(runtimeAssembly.GetSimpleName(), StringComparison.InvariantCultureIgnoreCase))
{
throw new InvalidOperationException(SR.Argument_CustomAssemblyLoadContextRequestedNameMismatch);
}

return assembly;
return runtimeAssembly;
jkotas marked this conversation as resolved.
Show resolved Hide resolved
}

private Assembly? ResolveUsingLoad(AssemblyName assemblyName)
private RuntimeAssembly? ResolveUsingLoad(AssemblyName assemblyName)
{
string? simpleName = assemblyName.Name;
Assembly? assembly = Load(assemblyName);

if (assembly != null)
{
assembly = ValidateAssemblyNameWithSimpleName(assembly, simpleName);
}
Assembly? assembly = Load(assemblyName);

return assembly;
return (assembly != null) ? ValidateAssemblyNameWithSimpleName(assembly, simpleName) : null;
}

private Assembly? ResolveUsingEvent(AssemblyName assemblyName)
private RuntimeAssembly? ResolveUsingEvent(AssemblyName assemblyName)
{
string? simpleName = assemblyName.Name;

// Invoke the Resolving event callbacks if wired up
Assembly? assembly = GetFirstResolvedAssemblyFromResolvingEvent(assemblyName);
if (assembly != null)
{
assembly = ValidateAssemblyNameWithSimpleName(assembly, simpleName);
}

return assembly;
return (assembly != null) ? ValidateAssemblyNameWithSimpleName(assembly, simpleName) : null;
}

// This method is called by the VM.
Expand Down Expand Up @@ -760,7 +748,7 @@ internal static void InvokeAssemblyLoadEvent(Assembly assembly)
Justification = "Satellite assemblies have no code in them and loading is not a problem")]
[UnconditionalSuppressMessage("SingleFile", "IL3000: Avoid accessing Assembly file path when publishing as a single file",
Justification = "This call is fine because native call runs before this and checks BindSatelliteResourceFromBundle")]
private Assembly? ResolveSatelliteAssembly(AssemblyName assemblyName)
private RuntimeAssembly? ResolveSatelliteAssembly(AssemblyName assemblyName)
{
// Called by native runtime when CultureName is not empty
Debug.Assert(assemblyName.CultureName?.Length > 0);
Expand All @@ -772,7 +760,7 @@ internal static void InvokeAssemblyLoadEvent(Assembly assembly)

string parentAssemblyName = assemblyName.Name.Substring(0, assemblyName.Name.Length - SatelliteSuffix.Length);

Assembly parentAssembly = LoadFromAssemblyName(new AssemblyName(parentAssemblyName));
RuntimeAssembly parentAssembly = (RuntimeAssembly)LoadFromAssemblyName(new AssemblyName(parentAssemblyName));

AssemblyLoadContext parentALC = GetLoadContext(parentAssembly)!;

Expand All @@ -795,7 +783,7 @@ internal static void InvokeAssemblyLoadEvent(Assembly assembly)
exists = FileSystem.FileExists(assemblyPath);
}

Assembly? asm = exists ? parentALC.LoadFromAssemblyPath(assemblyPath) : null;
RuntimeAssembly? asm = exists ? (RuntimeAssembly?)parentALC.LoadFromAssemblyPath(assemblyPath) : null;
#if CORECLR
if (IsTracingEnabled())
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.IO;
using System.Linq;
using System.Reflection;
using System.Reflection.Emit;
using System.Threading.Tasks;

namespace System.Runtime.Loader.Tests
Expand Down Expand Up @@ -233,5 +234,22 @@ public static void SubclassAssemblyLoadContext_Properties()
Assert.Contains(alc, AssemblyLoadContext.All);
Assert.Empty(alc.Assemblies);
}

class RefEmitLoadContext : AssemblyLoadContext
{
protected override Assembly? Load(AssemblyName assemblyName)
{
return AssemblyBuilder.DefineDynamicAssembly(assemblyName, AssemblyBuilderAccess.Run);
jkotas marked this conversation as resolved.
Show resolved Hide resolved
}
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsReflectionEmitSupported))]
[ActiveIssue("https://github.com/dotnet/runtime/issues/31804", TestRuntimes.Mono)]
public static void LoadRefEmitAssembly()
jkotas marked this conversation as resolved.
Show resolved Hide resolved
{
AssemblyLoadContext alc = new RefEmitLoadContext();
Exception error = Assert.Throws<FileLoadException>(() => alc.LoadFromAssemblyName(new AssemblyName("MyAssembly")));
Assert.IsType<InvalidOperationException>(error.InnerException);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -136,28 +136,6 @@ public override Module ManifestModule
// TODO: consider a dedicated icall instead
public override bool IsCollectible => AssemblyLoadContext.GetLoadContext((Assembly)this)!.IsCollectible;

internal static AssemblyName? CreateAssemblyName(string assemblyString, out RuntimeAssembly? assemblyFromResolveEvent)
jkotas marked this conversation as resolved.
Show resolved Hide resolved
{
ArgumentNullException.ThrowIfNull(assemblyString);

if ((assemblyString.Length == 0) ||
(assemblyString[0] == '\0'))
throw new ArgumentException(SR.Format_StringZeroLength);

assemblyFromResolveEvent = null;
try
{
return new AssemblyName(assemblyString);
}
catch (Exception)
{
assemblyFromResolveEvent = (RuntimeAssembly?)AssemblyLoadContext.DoAssemblyResolve(assemblyString);
if (assemblyFromResolveEvent == null)
throw new FileLoadException(assemblyString);
return null;
}
}

[MethodImplAttribute(MethodImplOptions.InternalCall)]
private static extern void GetManifestResourceNames(QCallAssembly assembly_h, ObjectHandleOnStack res);

Expand Down
Loading
Loading