From: Jan Kotas Date: Wed, 18 Apr 2018 22:35:48 +0000 (-0700) Subject: Delete RuntimeMethodHandle.GetSecurityFlag (#17643) X-Git-Tag: accepted/tizen/unified/20190422.045933~2285 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=686eb68c17d9ba1a45bc0cb3ceb7f3d6e343f6ff;p=platform%2Fupstream%2Fcoreclr.git Delete RuntimeMethodHandle.GetSecurityFlag (#17643) CAS leftover --- diff --git a/src/mscorlib/src/System/Reflection/INVOCATION_FLAGS.cs b/src/mscorlib/src/System/Reflection/INVOCATION_FLAGS.cs index b097b8f..eb28f4e 100644 --- a/src/mscorlib/src/System/Reflection/INVOCATION_FLAGS.cs +++ b/src/mscorlib/src/System/Reflection/INVOCATION_FLAGS.cs @@ -22,7 +22,7 @@ namespace System.Reflection // because field and method are different we can reuse the same bits // method INVOCATION_FLAGS_IS_CTOR = 0x00000010, - INVOCATION_FLAGS_RISKY_METHOD = 0x00000020, + /* unused 0x00000020 */ /* unused 0x00000040 */ INVOCATION_FLAGS_IS_DELEGATE_CTOR = 0x00000080, INVOCATION_FLAGS_CONTAINS_STACK_POINTERS = 0x00000100, diff --git a/src/mscorlib/src/System/Reflection/RuntimeConstructorInfo.cs b/src/mscorlib/src/System/Reflection/RuntimeConstructorInfo.cs index 9f7d79e..ad9a1a4 100644 --- a/src/mscorlib/src/System/Reflection/RuntimeConstructorInfo.cs +++ b/src/mscorlib/src/System/Reflection/RuntimeConstructorInfo.cs @@ -52,8 +52,9 @@ namespace System.Reflection } else { - // this should be an invocable method, determine the other flags that participate in invocation - invocationFlags |= RuntimeMethodHandle.GetSecurityFlags(this); + // Check for byref-like types + if (declaringType != null && declaringType.IsByRefLike) + invocationFlags |= INVOCATION_FLAGS.INVOCATION_FLAGS_CONTAINS_STACK_POINTERS; // Check for attempt to create a delegate class. if (typeof(Delegate).IsAssignableFrom(DeclaringType)) diff --git a/src/mscorlib/src/System/Reflection/RuntimeMethodInfo.cs b/src/mscorlib/src/System/Reflection/RuntimeMethodInfo.cs index 0d5d2de..692bfa3 100644 --- a/src/mscorlib/src/System/Reflection/RuntimeMethodInfo.cs +++ b/src/mscorlib/src/System/Reflection/RuntimeMethodInfo.cs @@ -50,8 +50,9 @@ namespace System.Reflection } else { - // this should be an invocable method, determine the other flags that participate in invocation - invocationFlags = RuntimeMethodHandle.GetSecurityFlags(this); + // Check for byref-like types + if ((declaringType != null && declaringType.IsByRefLike) || ReturnType.IsByRefLike) + invocationFlags |= INVOCATION_FLAGS.INVOCATION_FLAGS_CONTAINS_STACK_POINTERS; } m_invocationFlags = invocationFlags | INVOCATION_FLAGS.INVOCATION_FLAGS_INITIALIZED; diff --git a/src/mscorlib/src/System/RuntimeHandles.cs b/src/mscorlib/src/System/RuntimeHandles.cs index 80aec73..a4344f3 100644 --- a/src/mscorlib/src/System/RuntimeHandles.cs +++ b/src/mscorlib/src/System/RuntimeHandles.cs @@ -889,17 +889,6 @@ namespace System [MethodImplAttribute(MethodImplOptions.InternalCall)] internal static extern object InvokeMethod(object target, object[] arguments, Signature sig, bool constructor, bool wrapExceptions); - #region Private Invocation Helpers - internal static INVOCATION_FLAGS GetSecurityFlags(IRuntimeMethodInfo handle) - { - return (INVOCATION_FLAGS)RuntimeMethodHandle.GetSpecialSecurityFlags(handle); - } - - [MethodImplAttribute(MethodImplOptions.InternalCall)] - static internal extern uint GetSpecialSecurityFlags(IRuntimeMethodInfo method); - - #endregion - [DllImport(JitHelpers.QCall, CharSet = CharSet.Unicode)] private static extern void GetMethodInstantiation(RuntimeMethodHandleInternal method, ObjectHandleOnStack types, bool fAsRuntimeTypeArray); diff --git a/src/vm/comdelegate.cpp b/src/vm/comdelegate.cpp index 2927daf..9ef972e 100644 --- a/src/vm/comdelegate.cpp +++ b/src/vm/comdelegate.cpp @@ -3221,11 +3221,6 @@ MethodDesc* COMDelegate::GetDelegateCtor(TypeHandle delegateType, MethodDesc *pT } #endif - // Devdiv bug 296229: if the target method is dangerous, forcing the delegate creation to go through the - // slow path where we will do a demand to ensure security. - if (InvokeUtil::IsDangerousMethod(pTargetMethod)) - return NULL; - // DELEGATE KINDS TABLE // // _target _methodPtr _methodPtrAux _invocationList _invocationCount diff --git a/src/vm/dangerousapis.h b/src/vm/dangerousapis.h deleted file mode 100644 index 5168613..0000000 --- a/src/vm/dangerousapis.h +++ /dev/null @@ -1,57 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. -// - -// -//////////////////////////////////////////////////////////////////////////////// -// This header file defines the list of dangerous APIs and -// is used by InvokeUtil::IsDangerousMethod. -// Dangerous APIs are the APIs that make security decisions based on the result -// of a stack walk. When these APIs are invoked through reflection or delegate -// the stack walker can be easily confused, resulting in security holes. -//////////////////////////////////////////////////////////////////////////////// - -#ifndef API_NAMES -#define API_NAMES(...) __VA_ARGS__ -#endif // !API_NAMES - -// ToString is never dangerous but we include it on the Runtime*Info types because of a JScript.Net compat issue. -// JScript.Net tries to invoke these ToString APIs when a Runtime*Info object is compared to another object of a different type (e.g. a string). -// This used to cause a SecurityException in partial trust (which JScript catches) because the API was considered dangerous. -// Now this causes a MethodAccessException in partial trust because the API is inaccessible. So we add them back to the "dangerous" API -// list to maintain compatibility. See Devdiv bug 419443 for details. -DEFINE_DANGEROUS_API(APP_DOMAIN, API_NAMES("CreateInstance", "CreateComInstanceFrom", "CreateInstanceAndUnwrap", "CreateInstanceFrom", "CreateInstanceFromAndUnwrap ", "DefineDynamicAssembly", "Load")) -DEFINE_DANGEROUS_API(ASSEMBLYBASE, API_NAMES("CreateInstance", "Load")) -DEFINE_DANGEROUS_API(ASSEMBLY, API_NAMES("CreateInstance", "Load")) -DEFINE_DANGEROUS_API(ASSEMBLY_BUILDER, API_NAMES("CreateInstance", "DefineDynamicAssembly", "DefineDynamicModule")) -DEFINE_DANGEROUS_API(INTERNAL_ASSEMBLY_BUILDER, API_NAMES("CreateInstance")) -DEFINE_DANGEROUS_API(METHOD_BASE, API_NAMES("Invoke")) -DEFINE_DANGEROUS_API(CONSTRUCTOR_INFO, API_NAMES("Invoke", \ - "System.Runtime.InteropServices._ConstructorInfo.Invoke_2", \ - "System.Runtime.InteropServices._ConstructorInfo.Invoke_3", \ - "System.Runtime.InteropServices._ConstructorInfo.Invoke_4", \ - "System.Runtime.InteropServices._ConstructorInfo.Invoke_5")) -DEFINE_DANGEROUS_API(CONSTRUCTOR, API_NAMES("Invoke", "ToString")) -DEFINE_DANGEROUS_API(METHOD_INFO, API_NAMES("CreateDelegate", "Invoke")) -DEFINE_DANGEROUS_API(METHOD, API_NAMES("CreateDelegate", "Invoke", "ToString")) -DEFINE_DANGEROUS_API(DYNAMICMETHOD, API_NAMES("CreateDelegate", "Invoke", ".ctor")) -DEFINE_DANGEROUS_API(TYPE, API_NAMES("InvokeMember")) -DEFINE_DANGEROUS_API(CLASS, API_NAMES("InvokeMember", "ToString")) -DEFINE_DANGEROUS_API(TYPE_DELEGATOR, API_NAMES("InvokeMember")) -DEFINE_DANGEROUS_API(RT_FIELD_INFO, API_NAMES("GetValue", "SetValue", "ToString")) -DEFINE_DANGEROUS_API(FIELD_INFO, API_NAMES("GetValue", "SetValue")) -DEFINE_DANGEROUS_API(FIELD, API_NAMES("GetValue", "SetValue", "ToString")) -DEFINE_DANGEROUS_API(PROPERTY_INFO, API_NAMES("GetValue", "SetValue")) -DEFINE_DANGEROUS_API(PROPERTY, API_NAMES("GetValue", "SetValue", "ToString")) -DEFINE_DANGEROUS_API(EVENT_INFO, API_NAMES("AddEventHandler", "RemoveEventHandler")) -DEFINE_DANGEROUS_API(EVENT, API_NAMES("AddEventHandler", "RemoveEventHandler", "ToString")) -DEFINE_DANGEROUS_API(RESOURCE_MANAGER, API_NAMES("GetResourceSet", "InternalGetResourceSet", ".ctor")) - - - - - - - - diff --git a/src/vm/ecalllist.h b/src/vm/ecalllist.h index 66d2a3e..c4f5a0d 100644 --- a/src/vm/ecalllist.h +++ b/src/vm/ecalllist.h @@ -345,7 +345,6 @@ FCFuncStart(gRuntimeMethodHandle) QCFuncElement("Destroy", RuntimeMethodHandle::Destroy) FCFuncElement("GetResolver", RuntimeMethodHandle::GetResolver) FCFuncElement("GetLoaderAllocator", RuntimeMethodHandle::GetLoaderAllocator) - FCFuncElement("GetSpecialSecurityFlags", ReflectionInvocation::GetSpecialSecurityFlags) FCFuncEnd() FCFuncStart(gCOMDefaultBinderFuncs) diff --git a/src/vm/invokeutil.cpp b/src/vm/invokeutil.cpp index 823cefa..3672cc2 100644 --- a/src/vm/invokeutil.cpp +++ b/src/vm/invokeutil.cpp @@ -1712,95 +1712,3 @@ AccessCheckOptions::AccessCheckType InvokeUtil::GetInvocationAccessCheckType(BOO } #endif // CROSSGEN_COMPILE - -struct DangerousAPIEntry -{ - BinderClassID classID; - const LPCSTR *pszAPINames; - DWORD cAPINames; -}; - -#define DEFINE_DANGEROUS_API(classID, szAPINames) static const LPCSTR g__ ## classID ## __DangerousAPIs[] = { szAPINames }; -#include "dangerousapis.h" -#undef DEFINE_DANGEROUS_API - -#define DEFINE_DANGEROUS_API(classID, szAPINames) { CLASS__ ## classID, g__ ## classID ## __DangerousAPIs, NumItems(g__ ## classID ## __DangerousAPIs)}, -static const DangerousAPIEntry DangerousAPIs[] = -{ -#include "dangerousapis.h" -}; -#undef DEFINE_DANGEROUS_API - -/*static*/ -bool InvokeUtil::IsDangerousMethod(MethodDesc *pMD) -{ - CONTRACTL { - THROWS; - GC_TRIGGERS; - MODE_ANY; - } - CONTRACTL_END; - - MethodTable *pMT = pMD->GetMethodTable(); - - if (pMT->GetModule()->IsSystem()) - { - // All methods on these types are considered dangerous - static const BinderClassID dangerousTypes[] = { - CLASS__TYPE_HANDLE, - CLASS__METHOD_HANDLE, - CLASS__FIELD_HANDLE, - CLASS__ACTIVATOR, - CLASS__DELEGATE, - CLASS__MULTICAST_DELEGATE, - CLASS__RUNTIME_HELPERS - }; - - - static bool fInited = false; - - if (!VolatileLoad(&fInited)) - { - // Make sure all types are loaded so that we can use faster GetExistingClass() - for (unsigned i = 0; i < NumItems(dangerousTypes); i++) - { - MscorlibBinder::GetClass(dangerousTypes[i]); - } - - for (unsigned i = 0; i < NumItems(DangerousAPIs); i++) - { - MscorlibBinder::GetClass(DangerousAPIs[i].classID); - } - - VolatileStore(&fInited, true); - } - - for (unsigned i = 0; i < NumItems(dangerousTypes); i++) - { - if (MscorlibBinder::GetExistingClass(dangerousTypes[i]) == pMT) - return true; - } - - for (unsigned i = 0; i < NumItems(DangerousAPIs); i++) - { - DangerousAPIEntry entry = DangerousAPIs[i]; - if (MscorlibBinder::GetExistingClass(entry.classID) == pMT) - { - LPCUTF8 szMethodName = pMD->GetName(); - for (unsigned j = 0; j < entry.cAPINames; j++) - { - if (strcmp(szMethodName, entry.pszAPINames[j]) == 0) - return true; - } - - break; - } - } - } - - // For reduce compat risks we treat non-ctors on DynamicMethod as safe. - if (pMT->IsDelegate() && pMD->IsCtor()) - return true; - - return false; -} diff --git a/src/vm/invokeutil.h b/src/vm/invokeutil.h index 99450d0..79ded1d 100644 --- a/src/vm/invokeutil.h +++ b/src/vm/invokeutil.h @@ -259,8 +259,6 @@ public: static AccessCheckOptions::AccessCheckType GetInvocationAccessCheckType(BOOL targetRemoted = FALSE); - static bool IsDangerousMethod(MethodDesc *pMD); - private: // Check accessability of a type or nested type. static void CheckAccessClass(RefSecContext *pCtx, diff --git a/src/vm/reflectioninvocation.cpp b/src/vm/reflectioninvocation.cpp index 515f173..2cc86e5 100644 --- a/src/vm/reflectioninvocation.cpp +++ b/src/vm/reflectioninvocation.cpp @@ -28,29 +28,6 @@ #include "dbginterface.h" #include "argdestination.h" -// these flags are defined in XXXInfo.cs and only those that are used are replicated here -#define INVOCATION_FLAGS_UNKNOWN 0x00000000 -#define INVOCATION_FLAGS_INITIALIZED 0x00000001 - -// it's used for both method and field to signify that no access is allowed -#define INVOCATION_FLAGS_NO_INVOKE 0x00000002 - -// #define unused 0x00000004 - -// because field and method are different we can reuse the same bits -//method -#define INVOCATION_FLAGS_IS_CTOR 0x00000010 -#define INVOCATION_FLAGS_RISKY_METHOD 0x00000020 -// #define unused 0x00000040 -#define INVOCATION_FLAGS_IS_DELEGATE_CTOR 0x00000080 -#define INVOCATION_FLAGS_CONTAINS_STACK_POINTERS 0x00000100 -// field -#define INVOCATION_FLAGS_SPECIAL_FIELD 0x00000010 -#define INVOCATION_FLAGS_FIELD_SPECIAL_CAST 0x00000020 - -// temporary flag used for flagging invocation of method vs ctor -#define INVOCATION_FLAGS_CONSTRUCTOR_INVOKE 0x10000000 - /**************************************************************************/ /* if the type handle 'th' is a byref to a nullable type, return the type handle to the nullable type in the byref. Otherwise return @@ -646,56 +623,6 @@ FCIMPL2(FC_BOOL_RET, RuntimeTypeHandle::IsInstanceOfType, ReflectClassBaseObject } FCIMPLEND -FCIMPL1(DWORD, ReflectionInvocation::GetSpecialSecurityFlags, ReflectMethodObject *pMethodUNSAFE) { - CONTRACTL { - FCALL_CHECK; - } - CONTRACTL_END; - - DWORD dwFlags = 0; - - struct - { - REFLECTMETHODREF refMethod; - } - gc; - - gc.refMethod = (REFLECTMETHODREF)ObjectToOBJECTREF(pMethodUNSAFE); - - if (!gc.refMethod) - FCThrowRes(kArgumentNullException, W("Arg_InvalidHandle")); - - MethodDesc* pMethod = gc.refMethod->GetMethod(); - - HELPER_METHOD_FRAME_BEGIN_RET_PROTECT(gc); - - // this is an information that is critical for ctors, otherwise is not important - // we get it here anyway to simplify code - MethodTable *pMT = pMethod->GetMethodTable(); - _ASSERTE(pMT); - - // We should also check the return type here. - // Is there an easier way to get the return type of a method? - MetaSig metaSig(pMethod); - TypeHandle retTH = metaSig.GetRetTypeHandleThrowing(); - MethodTable *pRetMT = retTH.GetMethodTable(); - - // If either the declaring type or the return type contains stack pointers (ByRef or typedbyref), - // the type cannot be boxed and thus cannot be invoked through reflection invocation. - if ( pMT->IsByRefLike() || (pRetMT != NULL && pRetMT->IsByRefLike()) ) - dwFlags |= INVOCATION_FLAGS_CONTAINS_STACK_POINTERS; - - // Is this a call to a potentially dangerous method? (If so, we're going - // to demand additional permission). - if (InvokeUtil::IsDangerousMethod(pMethod)) - dwFlags |= INVOCATION_FLAGS_RISKY_METHOD; - - HELPER_METHOD_FRAME_END(); - return dwFlags; -} -FCIMPLEND - - /****************************************************************************/ /* boxed Nullable are represented as a boxed T, so there is no unboxed Nullable inside to point at by reference. Because of this a byref diff --git a/src/vm/reflectioninvocation.h b/src/vm/reflectioninvocation.h index 80b861f..64234d7 100644 --- a/src/vm/reflectioninvocation.h +++ b/src/vm/reflectioninvocation.h @@ -76,7 +76,6 @@ public: static FCDECL2_IV(Object*, CreateEnum, ReflectClassBaseObject *pTypeUNSAFE, INT64 value); // helper fcalls for invocation - static FCDECL1(DWORD, GetSpecialSecurityFlags, ReflectMethodObject *pMethodUNSAFE); static FCDECL2(FC_BOOL_RET, CanValueSpecialCast, ReflectClassBaseObject *valueType, ReflectClassBaseObject *targetType); static FCDECL3(Object*, AllocateValueType, ReflectClassBaseObject *targetType, Object *valueUNSAFE, CLR_BOOL fForceTypeChange); diff --git a/src/vm/runtimehandles.cpp b/src/vm/runtimehandles.cpp index de9505a..2aba21f 100644 --- a/src/vm/runtimehandles.cpp +++ b/src/vm/runtimehandles.cpp @@ -114,12 +114,6 @@ static BOOL CheckCAVisibilityFromDecoratedType(MethodTable* pCAMT, MethodDesc* p if (pCACtor != NULL) { - // Allowing a dangerous method to be called in custom attribute instantiation is, well, dangerous. - // E.g. a malicious user can craft a custom attribute record that fools us into creating a DynamicMethod - // object attached to typeof(System.Reflection.CustomAttribute) and thus gain access to mscorlib internals. - if (InvokeUtil::IsDangerousMethod(pCACtor)) - return FALSE; - _ASSERTE(pCACtor->IsCtor()); dwAttr = pCACtor->GetAttrs();