Skip to content

Commit

Permalink
[mono] Implement IgnoresAccessChecksToAttribute support (#48558)
Browse files Browse the repository at this point in the history 
* Implement IgnoresAccessChecksToAttribute support

   System.Runtime.CompilerServices.IgnoresAccessChecksToAttribute is like the reverse of InternalsVisibleTo - it's added to an assembly to indicate that that assembly can call non-public methods from the specified assembly.

   The main difference is that while InternalsVisibleTo allows access to `internal` members, IgnoresAccessChecksTo also allows access to `private` members.

   The interesting thing is that the DynamicProxy dynamically generates the attribute, so it is not explicitly declared in CoreLib.

   Fixes #47989

* Add System.Reflection.Emit regression tests that throw MethodAccessException

   check that calling private/internal methods in another assembly throws.
   check that calling a private method in the same assembly throws.
  • Loading branch information
@lambdageek
lambdageek authored Feb 27, 2021
1 parent d2f7522 commit 9102b07
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 23 deletions.
6 changes: 0 additions & 6 deletions src/libraries/System.Reflection.DispatchProxy/tests/DispatchProxyTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,28 +137,24 @@ public static void Create_Using_BaseType_Without_Default_Ctor_Throws_ArgumentExc
}

[Fact]
[SkipOnMono("https://github.com/dotnet/runtime/issues/47989")]
public static void Create_Using_PrivateProxy()
{
Assert.NotNull(TestType_PrivateProxy.Proxy<TestType_IHelloService>());
}

[Fact]
[SkipOnMono("https://github.com/dotnet/runtime/issues/47989")]
public static void Create_Using_PrivateProxyAndInternalService()
{
Assert.NotNull(TestType_PrivateProxy.Proxy<TestType_InternalInterfaceService>());
}

[Fact]
[SkipOnMono("https://github.com/dotnet/runtime/issues/47989")]
public static void Create_Using_PrivateProxyAndInternalServiceWithExternalGenericArgument()
{
Assert.NotNull(TestType_PrivateProxy.Proxy<TestType_InternalInterfaceWithNonPublicExternalGenericArgument>());
}

[Fact]
[SkipOnMono("https://github.com/dotnet/runtime/issues/47989")]
public static void Create_Using_InternalProxy()
{
Assert.NotNull(DispatchProxy.Create<TestType_InternalInterfaceService, InternalInvokeProxy>());
Expand All @@ -171,7 +167,6 @@ public static void Create_Using_ExternalNonPublicService()
}

[Fact]
[SkipOnMono("https://github.com/dotnet/runtime/issues/47989")]
public static void Create_Using_InternalProxyWithExternalNonPublicBaseType()
{
Assert.NotNull(DispatchProxy.Create<TestType_IHelloService, TestType_InternalProxyInternalBaseType>());
Expand All @@ -190,7 +185,6 @@ public static void Create_Using_InternalServiceWithGenericArgumentBeingNonPublic
}

[Fact]
[SkipOnMono("https://github.com/dotnet/runtime/issues/47989")]
public static void Create_Using_InternalProxyWithBaseTypeImplementingServiceWithgenericArgumentBeingNonPublicExternalService()
{
Assert.NotNull(DispatchProxy.Create<TestType_IHelloService, TestType_InternalProxyImplementingInterfaceWithGenericArgumentBeingNonPublicExternalType>());
Expand Down
86 changes: 86 additions & 0 deletions src/libraries/System.Reflection.Emit/tests/AssemblyBuilderTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,5 +340,91 @@ private static void VerifyAssemblyBuilder(AssemblyBuilder assembly, AssemblyName
Assert.Empty(assembly.DefinedTypes);
Assert.Empty(assembly.GetTypes());
}

private static void SamplePrivateMethod ()
{
}

internal static void SampleInternalMethod ()
{
}

[Fact]
void Invoke_Private_CrossAssembly_ThrowsMethodAccessException()
{
TypeBuilder tb = Helpers.DynamicType(TypeAttributes.Public);
var mb = tb.DefineMethod ("MyMethod", MethodAttributes.Public | MethodAttributes.Static, typeof(void), new Type[] { });

var ilg = mb.GetILGenerator ();

var callee = typeof (AssemblyTests).GetMethod ("SamplePrivateMethod", BindingFlags.Static | BindingFlags.NonPublic);

ilg.Emit (OpCodes.Call, callee);
ilg.Emit (OpCodes.Ret);

var ty = tb.CreateType ();

var mi = ty.GetMethod ("MyMethod", BindingFlags.Static | BindingFlags.Public);

var d = (Action) mi.CreateDelegate (typeof(Action));

Assert.Throws<MethodAccessException>(() => d ());
}

[Fact]
void Invoke_Internal_CrossAssembly_ThrowsMethodAccessException()
{
TypeBuilder tb = Helpers.DynamicType(TypeAttributes.Public);
var mb = tb.DefineMethod ("MyMethod", MethodAttributes.Public | MethodAttributes.Static, typeof(void), new Type[] { });

var ilg = mb.GetILGenerator ();

var callee = typeof (AssemblyTests).GetMethod ("SampleInternalMethod", BindingFlags.Static | BindingFlags.NonPublic);

ilg.Emit (OpCodes.Call, callee);
ilg.Emit (OpCodes.Ret);

var ty = tb.CreateType ();

var mi = ty.GetMethod ("MyMethod", BindingFlags.Static | BindingFlags.Public);

var d = (Action) mi.CreateDelegate (typeof(Action));

Assert.Throws<MethodAccessException>(() => d ());
}

[Fact]
void Invoke_Private_SameAssembly_ThrowsMethodAccessException()
{
ModuleBuilder modb = Helpers.DynamicModule();

string calleeName = "PrivateMethod";

TypeBuilder tbCalled = modb.DefineType ("CalledClass", TypeAttributes.Public);
var mbCalled = tbCalled.DefineMethod (calleeName, MethodAttributes.Private | MethodAttributes.Static);
mbCalled.GetILGenerator().Emit (OpCodes.Ret);

var tyCalled = tbCalled.CreateType();
var callee = tyCalled.GetMethod (calleeName, BindingFlags.NonPublic | BindingFlags.Static);

TypeBuilder tb = modb.DefineType("CallerClass", TypeAttributes.Public);
var mb = tb.DefineMethod ("MyMethod", MethodAttributes.Public | MethodAttributes.Static, typeof(void), new Type[] { });

var ilg = mb.GetILGenerator ();

ilg.Emit (OpCodes.Call, callee);
ilg.Emit (OpCodes.Ret);

var ty = tb.CreateType ();

var mi = ty.GetMethod ("MyMethod", BindingFlags.Static | BindingFlags.Public);

var d = (Action) mi.CreateDelegate (typeof(Action));

Assert.Throws<MethodAccessException>(() => d ());
}



}
}
42 changes: 28 additions & 14 deletions src/mono/mono/metadata/assembly.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2080,8 +2080,9 @@ mono_assembly_request_open (const char *filename, const MonoAssemblyOpenRequest
}

static void
free_item (gpointer val, gpointer user_data)
free_assembly_name_item (gpointer val, gpointer user_data)
{
mono_assembly_name_free_internal ((MonoAssemblyName *)val);
g_free (val);
}

Expand All @@ -2105,7 +2106,6 @@ mono_assembly_load_friends (MonoAssembly* ass)
ERROR_DECL (error);
int i;
MonoCustomAttrInfo* attrs;
GSList *list;

if (ass->friend_assembly_names_inited)
return;
Expand All @@ -2126,7 +2126,9 @@ mono_assembly_load_friends (MonoAssembly* ass)
}
mono_assemblies_unlock ();

list = NULL;
GSList *visible_list = NULL;
GSList *ignores_list = NULL;

/*
* We build the list outside the assemblies lock, the worse that can happen
* is that we'll need to free the allocated list.
Expand All @@ -2138,7 +2140,16 @@ mono_assembly_load_friends (MonoAssembly* ass)
uint32_t data_length;
gchar *data_with_terminator;
/* Do some sanity checking */
if (!attr->ctor || attr->ctor->klass != mono_class_try_get_internals_visible_class ())
if (!attr->ctor)
continue;
gboolean has_visible = FALSE;
gboolean has_ignores = FALSE;
has_visible = attr->ctor->klass == mono_class_try_get_internals_visible_class ();
/* IgnoresAccessChecksToAttribute is dynamically generated, so it's not necessarily in CoreLib */
/* FIXME: should we only check for it in dynamic modules? */
has_ignores = (!strcmp ("IgnoresAccessChecksToAttribute", m_class_get_name (attr->ctor->klass)) &&
!strcmp ("System.Runtime.CompilerServices", m_class_get_name_space (attr->ctor->klass)));
if (!has_visible && !has_ignores)
continue;
if (attr->data_size < 4)
continue;
Expand All @@ -2152,7 +2163,10 @@ mono_assembly_load_friends (MonoAssembly* ass)
aname = g_new0 (MonoAssemblyName, 1);
/*g_print ("friend ass: %s\n", data);*/
if (mono_assembly_name_parse_full (data_with_terminator, aname, TRUE, NULL, NULL)) {
list = g_slist_prepend (list, aname);
if (has_visible)
visible_list = g_slist_prepend (visible_list, aname);
if (has_ignores)
ignores_list = g_slist_prepend (ignores_list, aname);
} else {
g_free (aname);
}
Expand All @@ -2163,11 +2177,14 @@ mono_assembly_load_friends (MonoAssembly* ass)
mono_assemblies_lock ();
if (ass->friend_assembly_names_inited) {
mono_assemblies_unlock ();
g_slist_foreach (list, free_item, NULL);
g_slist_free (list);
g_slist_foreach (visible_list, free_assembly_name_item, NULL);
g_slist_free (visible_list);
g_slist_foreach (ignores_list, free_assembly_name_item, NULL);
g_slist_free (ignores_list);
return;
}
ass->friend_assembly_names = list;
ass->friend_assembly_names = visible_list;
ass->ignores_checks_assembly_names = ignores_list;

/* Because of the double checked locking pattern above */
mono_memory_barrier ();
Expand Down Expand Up @@ -3318,7 +3335,6 @@ mono_assembly_release_gc_roots (MonoAssembly *assembly)
gboolean
mono_assembly_close_except_image_pools (MonoAssembly *assembly)
{
GSList *tmp;
g_return_val_if_fail (assembly != NULL, FALSE);

if (assembly == REFERENCE_MISSING)
Expand All @@ -3343,12 +3359,10 @@ mono_assembly_close_except_image_pools (MonoAssembly *assembly)
if (!mono_image_close_except_pools (assembly->image))
assembly->image = NULL;

for (tmp = assembly->friend_assembly_names; tmp; tmp = tmp->next) {
MonoAssemblyName *fname = (MonoAssemblyName *)tmp->data;
mono_assembly_name_free_internal (fname);
g_free (fname);
}
g_slist_foreach (assembly->friend_assembly_names, free_assembly_name_item, NULL);
g_slist_foreach (assembly->ignores_checks_assembly_names, free_assembly_name_item, NULL);
g_slist_free (assembly->friend_assembly_names);
g_slist_free (assembly->ignores_checks_assembly_names);
g_free (assembly->basedir);

MONO_PROFILER_RAISE (assembly_unloaded, (assembly));
Expand Down
25 changes: 22 additions & 3 deletions src/mono/mono/metadata/class.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5855,6 +5855,23 @@ is_valid_family_access (MonoClass *access_klass, MonoClass *member_klass, MonoCl
return TRUE;
}

static gboolean
ignores_access_checks_to (MonoAssembly *accessing, MonoAssembly *accessed)
{
if (!accessing || !accessed)
return FALSE;

mono_assembly_load_friends (accessing);
for (GSList *tmp = accessing->ignores_checks_assembly_names; tmp; tmp = tmp->next) {
MonoAssemblyName *victim = (MonoAssemblyName *)tmp->data;
if (!victim->name)
continue;
if (!g_ascii_strcasecmp (accessed->aname.name, victim->name))
return TRUE;
}
return FALSE;
}

static gboolean
can_access_internals (MonoAssembly *accessing, MonoAssembly* accessed)
{
Expand All @@ -5880,7 +5897,7 @@ can_access_internals (MonoAssembly *accessing, MonoAssembly* accessed)
}
return TRUE;
}
return FALSE;
return ignores_access_checks_to (accessing, accessed);
}

/*
Expand Down Expand Up @@ -5979,7 +5996,9 @@ can_access_type (MonoClass *access_klass, MonoClass *member_klass)
return member_klass_nested_in && can_access_type (access_klass, member_klass_nested_in);

case TYPE_ATTRIBUTE_NESTED_PRIVATE:
return is_nesting_type (member_klass, access_klass) && member_klass_nested_in && can_access_type (access_klass, member_klass_nested_in);
if (is_nesting_type (member_klass, access_klass) && member_klass_nested_in && can_access_type (access_klass, member_klass_nested_in))
return TRUE;
return ignores_access_checks_to (access_klass_assembly, member_klass_assembly);

case TYPE_ATTRIBUTE_NESTED_FAMILY:
return mono_class_has_parent_and_ignore_generics (access_klass, m_class_get_nested_in (member_klass));
Expand Down Expand Up @@ -6028,7 +6047,7 @@ can_access_member (MonoClass *access_klass, MonoClass *member_klass, MonoClass*
/* same compilation unit */
return m_class_get_image (access_klass) == member_klass_image;
case FIELD_ATTRIBUTE_PRIVATE:
return access_klass == member_klass;
return (access_klass == member_klass) || ignores_access_checks_to (access_klass_assembly, member_klass_image->assembly);
case FIELD_ATTRIBUTE_FAM_AND_ASSEM:
if (is_valid_family_access (access_klass, member_klass, context_klass) &&
can_access_internals (access_klass_assembly, member_klass_image->assembly))
Expand Down
1 change: 1 addition & 0 deletions src/mono/mono/metadata/metadata-internals.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ struct _MonoAssembly {
MonoAssemblyName aname;
MonoImage *image;
GSList *friend_assembly_names; /* Computed by mono_assembly_load_friends () */
GSList *ignores_checks_assembly_names; /* Computed by mono_assembly_load_friends () */
guint8 friend_assembly_names_inited;
guint8 dynamic;
MonoAssemblyContext context;
Expand Down

0 comments on commit 9102b07

Please sign in to comment.