diff options
author | Jan Kotas <jkotas@microsoft.com> | 2017-08-07 12:16:45 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-08-07 12:16:45 -0700 |
commit | 46ab1d132c9ad471d79afa20c188c2f9c85e5f20 (patch) | |
tree | 233ab93323e49cf4c5404e18304374a9faae74de /src/vm/comdelegate.cpp | |
parent | a9516dacd742ccaeae2820b89ad313a53d22d917 (diff) | |
download | coreclr-46ab1d132c9ad471d79afa20c188c2f9c85e5f20.tar.gz coreclr-46ab1d132c9ad471d79afa20c188c2f9c85e5f20.tar.bz2 coreclr-46ab1d132c9ad471d79afa20c188c2f9c85e5f20.zip |
Cleanup code access security from the unmanaged runtime (#13241)
Diffstat (limited to 'src/vm/comdelegate.cpp')
-rw-r--r-- | src/vm/comdelegate.cpp | 182 |
1 files changed, 16 insertions, 166 deletions
diff --git a/src/vm/comdelegate.cpp b/src/vm/comdelegate.cpp index cee0d8c08a..961a758750 100644 --- a/src/vm/comdelegate.cpp +++ b/src/vm/comdelegate.cpp @@ -22,7 +22,6 @@ #include "mdaassistants.h" #include "cgensys.h" #include "asmconstants.h" -#include "security.h" #include "virtualcallstub.h" #include "callingconvention.h" #include "customattribute.h" @@ -933,30 +932,6 @@ void COMDelegate::BindToMethod(DELEGATEREF *pRefThis, pExactMethodType, pTargetMethod->IsStatic() ? NULL : pInstanceMT, pTargetMethod); - - // Ask for skip verification if a delegate over a .ctor or .cctor is requested. - if (pTargetMethod->IsClassConstructorOrCtor()) - Security::SpecialDemand(SSWT_LATEBOUND_LINKDEMAND, SECURITY_SKIP_VER); - -#ifdef FEATURE_COMINTEROP - // Check if it's a COM object and if so, demand unmanaged code permission. - // <TODO> I think we need a target check here. Investigate. </TODO> - if (pTargetMethod && pTargetMethod->GetMethodTable()->IsComObjectType()) - Security::SpecialDemand(SSWT_DEMAND_FROM_NATIVE, SECURITY_UNMANAGED_CODE); -#endif // FEATURE_COMINTEROP - - // Devdiv bug 296229: dangerous methods are those that make security decisions based on - // the result of stack walks. When a delegate to such a method is invoked asynchronously - // the stackwalker will stop at the remoting code and consider the caller unmanaged code. - // Unmanaged code is allowed to bypass any security check. - if (InvokeUtil::IsDangerousMethod(pTargetMethod)) - Security::SpecialDemand(SSWT_LATEBOUND_LINKDEMAND, REFLECTION_MEMBER_ACCESS); - - // Check whether the creator of the delegate lives in the same assembly as the target method. If not, and they aren't fully - // trusted, we have to make this delegate a secure wrapper and allocate a new inner delegate to represent the real target. - MethodDesc *pCreatorMethod = sCtx.GetCallerMethod(); - if (NeedsSecureDelegate(pCreatorMethod, sCtx.GetCallerDomain(), pTargetMethod)) - refRealDelegate = CreateSecureDelegate(*pRefThis, pCreatorMethod, pTargetMethod); } // If we didn't wrap the real delegate in a secure delegate then the real delegate is the one passed in. @@ -1511,8 +1486,7 @@ OBJECTREF COMDelegate::ConvertToDelegate(LPVOID pCallback, MethodTable* pMT) { GCX_PREEMP(); - DWORD dwStubFlags = pMT->ClassRequiresUnmanagedCodeCheck() ? NDIRECTSTUB_FL_HASDECLARATIVESECURITY : 0; - pMarshalStub = GetStubForInteropMethod(pMD, dwStubFlags, &(pClass->m_pForwardStubMD)); + pMarshalStub = GetStubForInteropMethod(pMD, 0, &(pClass->m_pForwardStubMD)); // Save this new stub on the DelegateEEClass. EnsureWritablePages(dac_cast<PVOID>(&pClass->m_pMarshalStub), sizeof(PCODE)); @@ -1633,9 +1607,6 @@ OBJECTREF COMDelegate::ConvertWinRTInterfaceToDelegate(IUnknown *pIdentity, Meth DWORD dwStubFlags = NDIRECTSTUB_FL_COM | NDIRECTSTUB_FL_WINRT | NDIRECTSTUB_FL_WINRTDELEGATE; - if (pMT->ClassRequiresUnmanagedCodeCheck()) - dwStubFlags |= NDIRECTSTUB_FL_HASDECLARATIVESECURITY; - pMarshalStub = GetStubForInteropMethod(pMD, dwStubFlags); // At this point we must have a non-NULL ComPlusCallInfo @@ -1737,9 +1708,6 @@ MethodDesc* COMDelegate::GetILStubMethodDesc(EEImplMethodDesc* pDelegateMD, DWOR dwStubFlags |= NDIRECTSTUB_FL_DELEGATE; } - if (pMT->ClassRequiresUnmanagedCodeCheck()) - dwStubFlags |= NDIRECTSTUB_FL_HASDECLARATIVESECURITY; - PInvokeStaticSigInfo sigInfo(pDelegateMD); return NDirect::CreateCLRToNativeILStub(&sigInfo, dwStubFlags, pDelegateMD); } @@ -1832,8 +1800,6 @@ FCIMPL3(PCODE, COMDelegate::AdjustTarget, Object* refThisUNSAFE, Object* targetU #ifdef FEATURE_COMINTEROP isComObject = pMTTarg->IsComObjectType(); - if (isComObject) - DoUnmanagedCodeAccessCheck(pMeth); #endif // FEATURE_COMINTEROP if (!pMT->IsTransparentProxy()) @@ -1971,18 +1937,7 @@ FCIMPL3(void, COMDelegate::DelegateConstruct, Object* refThisUNSAFE, Object* tar methodArgCount++; // count 'this' } - // do we need a secure delegate? - - // Devdiv bug 296229: dangerous methods are those that make security decisions based on - // the result of stack walks. When a delegate to such a method is invoked asynchronously - // the stackwalker will stop at the remoting code and consider the caller unmanaged code. - // Unmanaged code is allowed to bypass any security check. - if (InvokeUtil::IsDangerousMethod(pMeth)) - Security::SpecialDemand(SSWT_LATEBOUND_LINKDEMAND, REFLECTION_MEMBER_ACCESS); - - if (NeedsSecureDelegate(pCreatorMethod, GetAppDomain(), pMeth)) - gc.refThis = CreateSecureDelegate(gc.refThis, pCreatorMethod, pMeth); - else if (NeedsWrapperDelegate(pMeth)) + if (NeedsWrapperDelegate(pMeth)) gc.refThis = CreateSecureDelegate(gc.refThis, NULL, pMeth); if (pMeth->GetLoaderAllocator()->IsCollectible()) @@ -2033,8 +1988,6 @@ FCIMPL3(void, COMDelegate::DelegateConstruct, Object* refThisUNSAFE, Object* tar BOOL isComObject = false; #ifdef FEATURE_COMINTEROP isComObject = pMTTarg->IsComObjectType(); - if (isComObject) - DoUnmanagedCodeAccessCheck(pMeth); #endif // FEATURE_COMINTEROP if (!pMTTarg->IsTransparentProxy()) @@ -2125,56 +2078,6 @@ FCIMPL3(void, COMDelegate::DelegateConstruct, Object* refThisUNSAFE, Object* tar } FCIMPLEND - -#ifdef FEATURE_COMINTEROP -void COMDelegate::DoUnmanagedCodeAccessCheck(MethodDesc* pMeth) -{ - // Skip if SuppressUnmanagedCodePermission is present - if (pMeth->RequiresLinktimeCheck()) - { - // Check whether this is actually a SuppressUnmanagedCodePermission attribute and - // if so, don't do a demand - { - return; - } - } - - // If this method is defined directly on an interface, get that interface - // Otherwise, from the class get the interface that this method is defined on. - // Based on this interface, skip the check if the interface is DispatchOnly or - // if the interface is defined in fully-trusted code. - if (pMeth->IsComPlusCall()) - { - ComPlusCallMethodDesc *pCMD = (ComPlusCallMethodDesc *)pMeth; - MethodTable* pMTItf = (pCMD->m_pComPlusCallInfo == NULL ? NULL : pCMD->m_pComPlusCallInfo->m_pInterfaceMT); - - // If the interface methodtable is null, then the ComPlusCallMethodDesc hasn't been set up yet. - if (pMTItf == NULL) - { - GCX_PREEMP(); - pMeth->DoPrestub(NULL); - pMTItf = ((ComPlusCallMethodDesc*)pMeth)->m_pComPlusCallInfo->m_pInterfaceMT; - } - else - { - pMTItf->CheckRestore(); - } - - if (pMTItf->GetComInterfaceType() == ifDispatch) - { - return; - } - else if (Security::CanCallUnmanagedCode(pMTItf->GetModule())) - { - return; - } - } - - Security::SpecialDemand(SSWT_DEMAND_FROM_NATIVE, SECURITY_UNMANAGED_CODE); -} -#endif // FEATURE_COMINTEROP - - MethodDesc *COMDelegate::GetMethodDesc(OBJECTREF orDelegate) { CONTRACTL @@ -2463,20 +2366,6 @@ FCIMPLEND #endif // CROSSGEN_COMPILE - -BOOL COMDelegate::NeedsSecureDelegate(MethodDesc* pCreatorMethod, AppDomain *pCreatorDomain, MethodDesc* pTargetMD) -{ - CONTRACTL - { - THROWS; - GC_TRIGGERS; - MODE_ANY; - } - CONTRACTL_END; - - return FALSE; -} - BOOL COMDelegate::NeedsWrapperDelegate(MethodDesc* pTargetMD) { LIMITED_METHOD_CONTRACT; @@ -3422,19 +3311,13 @@ MethodDesc* COMDelegate::GetDelegateCtor(TypeHandle delegateType, MethodDesc *pT if (!isStatic) methodArgCount++; // count 'this' MethodDesc *pCallerMethod = (MethodDesc*)pCtorData->pMethod; - BOOL needsSecureDelegate = NeedsSecureDelegate(pCallerMethod, GetAppDomain(), pTargetMethod); - if (!needsSecureDelegate && NeedsWrapperDelegate(pTargetMethod)) + if (NeedsWrapperDelegate(pTargetMethod)) { // If we need a wrapper even it is not a secure delegate, go through slow path return NULL; } - // If this is a secure delegate case, and the secure delegate would have a pointer to a collectible - // method in it, then use the slow path. This could be optimized with a set of fast paths. - if (needsSecureDelegate && (pCallerMethod->IsLCGMethod() || pCallerMethod->GetLoaderAllocator()->IsCollectible())) - return NULL; - // Force the slow path for nullable so that we can give the user an error in case were the verifier is not run. MethodTable* pMT = pTargetMethod->GetMethodTable(); if (!pTargetMethod->IsStatic() && Nullable::IsNullableType(pMT)) @@ -3486,10 +3369,6 @@ MethodDesc* COMDelegate::GetDelegateCtor(TypeHandle delegateType, MethodDesc *pT // Another is to pass a gchandle to the delegate ctor. This is fastest, but only works if we can predict the gc handle at this time. // We will use this for the non secure variants - // Collectible secure delegates can go down the slow path - if (isCollectible && needsSecureDelegate) - return NULL; - if (invokeArgCount == methodArgCount) { // case 2, 3, 6 @@ -3501,9 +3380,7 @@ MethodDesc* COMDelegate::GetDelegateCtor(TypeHandle delegateType, MethodDesc *pT if (!isStatic && pTargetMethod->IsVirtual() && !pTargetMethod->GetMethodTable()->IsValueType()) { // case 3 - if (needsSecureDelegate) - pRealCtor = MscorlibBinder::GetMethod(METHOD__MULTICAST_DELEGATE__CTOR_SECURE_VIRTUAL_DISPATCH); - else if (isCollectible) + if (isCollectible) pRealCtor = MscorlibBinder::GetMethod(METHOD__MULTICAST_DELEGATE__CTOR_COLLECTIBLE_VIRTUAL_DISPATCH); else pRealCtor = MscorlibBinder::GetMethod(METHOD__MULTICAST_DELEGATE__CTOR_VIRTUAL_DISPATCH); @@ -3511,9 +3388,7 @@ MethodDesc* COMDelegate::GetDelegateCtor(TypeHandle delegateType, MethodDesc *pT else { // case 2, 6 - if (needsSecureDelegate) - pRealCtor = MscorlibBinder::GetMethod(METHOD__MULTICAST_DELEGATE__CTOR_SECURE_OPENED); - else if (isCollectible) + if (isCollectible) pRealCtor = MscorlibBinder::GetMethod(METHOD__MULTICAST_DELEGATE__CTOR_COLLECTIBLE_OPENED); else pRealCtor = MscorlibBinder::GetMethod(METHOD__MULTICAST_DELEGATE__CTOR_OPENED); @@ -3527,13 +3402,7 @@ MethodDesc* COMDelegate::GetDelegateCtor(TypeHandle delegateType, MethodDesc *pT if (!pShuffleThunk) pShuffleThunk = SetupShuffleThunk(pDelMT, pTargetMethod); pCtorData->pArg3 = (void*)pShuffleThunk->GetEntryPoint(); - if (needsSecureDelegate) - { - // need to fill the info for the secure delegate - pCtorData->pArg4 = (void *)GetSecureInvoke(pDelegateInvoke); - pCtorData->pArg5 = pCallerMethod; - } - else if (isCollectible) + if (isCollectible) { pCtorData->pArg4 = pTargetMethodLoaderAllocator->GetLoaderAllocatorObjectHandle(); } @@ -3557,41 +3426,22 @@ MethodDesc* COMDelegate::GetDelegateCtor(TypeHandle delegateType, MethodDesc *pT (pTargetMethod->IsInterface() || (pTargetMethod->GetMethodTable()->IsValueType() && !pTargetMethod->IsUnboxingStub())); - if (needsSecureDelegate) - { - if (needsRuntimeInfo) - pRealCtor = MscorlibBinder::GetMethod(METHOD__MULTICAST_DELEGATE__CTOR_SECURE_RT_CLOSED); - else - { - if (!isStatic) - pRealCtor = MscorlibBinder::GetMethod(METHOD__MULTICAST_DELEGATE__CTOR_SECURE_CLOSED); - else - pRealCtor = MscorlibBinder::GetMethod(METHOD__MULTICAST_DELEGATE__CTOR_SECURE_CLOSED_STATIC); - } - - // need to fill the info for the secure delegate - pCtorData->pArg3 = (void *)GetSecureInvoke(pDelegateInvoke); - pCtorData->pArg4 = pCallerMethod; - } + if (needsRuntimeInfo) + pRealCtor = MscorlibBinder::GetMethod(METHOD__MULTICAST_DELEGATE__CTOR_RT_CLOSED); else { - if (needsRuntimeInfo) - pRealCtor = MscorlibBinder::GetMethod(METHOD__MULTICAST_DELEGATE__CTOR_RT_CLOSED); + if (!isStatic) + pRealCtor = MscorlibBinder::GetMethod(METHOD__MULTICAST_DELEGATE__CTOR_CLOSED); else { - if (!isStatic) - pRealCtor = MscorlibBinder::GetMethod(METHOD__MULTICAST_DELEGATE__CTOR_CLOSED); + if (isCollectible) + { + pRealCtor = MscorlibBinder::GetMethod(METHOD__MULTICAST_DELEGATE__CTOR_COLLECTIBLE_CLOSED_STATIC); + pCtorData->pArg3 = pTargetMethodLoaderAllocator->GetLoaderAllocatorObjectHandle(); + } else { - if (isCollectible) - { - pRealCtor = MscorlibBinder::GetMethod(METHOD__MULTICAST_DELEGATE__CTOR_COLLECTIBLE_CLOSED_STATIC); - pCtorData->pArg3 = pTargetMethodLoaderAllocator->GetLoaderAllocatorObjectHandle(); - } - else - { - pRealCtor = MscorlibBinder::GetMethod(METHOD__MULTICAST_DELEGATE__CTOR_CLOSED_STATIC); - } + pRealCtor = MscorlibBinder::GetMethod(METHOD__MULTICAST_DELEGATE__CTOR_CLOSED_STATIC); } } } |