Fix detection of static readonly field re-initialization via reflection#37849

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.

Sign up for GitHub

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

Merged
jkotas merged 3 commits into from
Jun 15, 2020
Merged

Fix detection of static readonly field re-initialization via reflection #37849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5af8287
Fix detection of static readonly field re-initialization via reflection
jkotas Jun 12, 2020
2a580ac
Apply suggestions from code review
jkotas Jun 15, 2020
c055567
Fix test disable
jkotas Jun 15, 2020
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
58 changes: 31 additions & 27 deletions src/coreclr/src/vm/invokeutil.cpp
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -941,11 +941,6 @@ void InvokeUtil::SetValidField(CorElementType fldType,
}
CONTRACTL_END;

// We don't allow setting the field of nullable<T> (hasValue and value)
// Because you can't independantly set them for this type.
if (!declaringType.IsNull() && Nullable::IsNullableType(declaringType.GetMethodTable()))
COMPlusThrow(kNotSupportedException);

// call the <cinit>
OBJECTREF Throwable = NULL;

Expand All@@ -954,27 +949,28 @@ void InvokeUtil::SetValidField(CorElementType fldType,
{
pDeclMT = declaringType.GetMethodTable();

// We don't allow setting the field of nullable<T> (hasValue and value)
// Because you can't independently set them for this type.
if (Nullable::IsNullableType(pDeclMT))
COMPlusThrow(kNotSupportedException);

if (pDeclMT->IsSharedByGenericInstantiations())
COMPlusThrow(kNotSupportedException, W("NotSupported_Type"));
}
else
{
pDeclMT = pField->GetModule()->GetGlobalMethodTable();
}

if (*pDomainInitialized == FALSE)
{
EX_TRY
{
if (declaringType.IsNull())
{
pField->GetModule()->GetGlobalMethodTable()->EnsureInstanceActive();
pField->GetModule()->GetGlobalMethodTable()->CheckRunClassInitThrowing();
}
else
{
pDeclMT->EnsureInstanceActive();
pDeclMT->CheckRunClassInitThrowing();

*pDomainInitialized = TRUE;
}
}
EX_CATCH_THROWABLE(&Throwable);
}
#ifdef _DEBUG
Expand All@@ -990,6 +986,18 @@ void InvokeUtil::SetValidField(CorElementType fldType,
GCPROTECT_END();
}

// Verify we're not trying to set the value of a static initonly field
// once the class has been initialized.
if (pField->IsStatic() && pDeclMT->IsClassInited() && IsFdInitOnly(pField->GetAttributes()))
{
DefineFullyQualifiedNameForClassW();
SString ssFieldName(SString::Utf8, pField->GetName());
COMPlusThrow(kFieldAccessException,
IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
ssFieldName.GetUnicode(),
GetFullyQualifiedNameForClassW(pDeclMT));
}

// Set the field
ARG_SLOT value;

Expand DownExpand Up@@ -1162,27 +1170,29 @@ OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJ
{
pDeclMT = declaringType.GetMethodTable();

// We don't allow getting the field just so we don't have more specical
// cases than we need to. Then we need at least the throw check to ensure
// we don't allow data corruption.
if (Nullable::IsNullableType(pDeclMT))
COMPlusThrow(kNotSupportedException);

if (pDeclMT->IsSharedByGenericInstantiations())
COMPlusThrow(kNotSupportedException, W("NotSupported_Type"));
}
else
{
pDeclMT = pField->GetModule()->GetGlobalMethodTable();
}

if (*pDomainInitialized == FALSE)
{
EX_TRY
{
if (declaringType.IsNull())
{
pField->GetModule()->GetGlobalMethodTable()->EnsureInstanceActive();
pField->GetModule()->GetGlobalMethodTable()->CheckRunClassInitThrowing();
}
else
{
pDeclMT->EnsureInstanceActive();
pDeclMT->CheckRunClassInitThrowing();

*pDomainInitialized = TRUE;
}
}
EX_CATCH_THROWABLE(&Throwable);
}
#ifdef _DEBUG
Expand All@@ -1199,12 +1209,6 @@ OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJ
GCPROTECT_END();
}

// We don't allow getting the field just so we don't have more specical
// cases than we need to. The we need at least the throw check to insure
// we don't allow data corruption, but
if (!declaringType.IsNull() && Nullable::IsNullableType(pDeclMT))
COMPlusThrow(kNotSupportedException);

CorElementType fieldElementType = pField->GetFieldType();

switch (fieldElementType) {
Expand Down
36 changes: 2 additions & 34 deletions src/coreclr/src/vm/reflectioninvocation.cpp
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -328,23 +328,6 @@ FCIMPL7(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Ob

HELPER_METHOD_FRAME_BEGIN_PROTECT(gc);

// Verify we're not trying to set the value of a static initonly field
// once the class has been initialized.
if (pFieldDesc->IsStatic())
{
MethodTable* pEnclosingMT = pFieldDesc->GetEnclosingMethodTable();
if (pEnclosingMT->IsClassInited() && IsFdInitOnly(pFieldDesc->GetAttributes()))
{
DefineFullyQualifiedNameForClassW();
SString ssFieldName(SString::Utf8, pFieldDesc->GetName());
COMPlusThrow(kFieldAccessException,
IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
ssFieldName.GetUnicode(),
GetFullyQualifiedNameForClassW(pEnclosingMT));
}
}

//TODO: cleanup this function
InvokeUtil::SetValidField(fieldType.GetSignatureCorElementType(), fieldType, pFieldDesc, &gc.target, &gc.value, declaringType, pDomainInitialized);

HELPER_METHOD_FRAME_END();
Expand DownExpand Up@@ -1582,7 +1565,6 @@ static void DirectObjectFieldSet(FieldDesc *pField, TypeHandle fieldType, TypeHa
// Validate the target/fld type relationship
InvokeUtil::ValidateObjectTarget(pField, enclosingType, &objref);

InvokeUtil::ValidField(fieldType, pValue);
InvokeUtil::SetValidField(pField->GetFieldType(), fieldType, pField, &objref, pValue, enclosingType, pDomainInitialized);
GCPROTECT_END();
}
Expand DownExpand Up@@ -1626,22 +1608,8 @@ FCIMPL5(void, RuntimeFieldHandle::SetValueDirect, ReflectFieldObject *pFieldUNSA
TypeHandle targetType = pTarget->type;
MethodTable *pEnclosingMT = contextType.GetMethodTable();

{
// Verify that the value passed can be widened into the target
InvokeUtil::ValidField(fieldType, &gc.oValue);

// Verify we're not trying to set the value of a static initonly field
// once the class has been initialized.
if (pField->IsStatic() && pEnclosingMT->IsClassInited() && IsFdInitOnly(pField->GetAttributes()))
{
DefineFullyQualifiedNameForClassW();
SString ssFieldName(SString::Utf8, pField->GetName());
COMPlusThrow(kFieldAccessException,
IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
ssFieldName.GetUnicode(),
GetFullyQualifiedNameForClassW(pEnclosingMT));
}
}
// Verify that the value passed can be widened into the target
InvokeUtil::ValidField(fieldType, &gc.oValue);

CLR_BOOL domainInitialized = FALSE;
if (pField->IsStatic() || !targetType.IsValueType()) {
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/tests/issues.targets
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1571,6 +1571,9 @@
<ExcludeList Include="$(XunitTestBinBase)/reflection/SetValue/TrySetReadonlyStaticField/**">
<Issue>needs triage</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/reflection/SetValue/TrySetReadonlyStaticField2/**">
<Issue>https://github.com/dotnet/runtime/issues/37899</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/Regressions/coreclr/15241/genericcontext/**">
<Issue>needs triage</Issue>
</ExcludeList>
Expand Down
51 changes: 51 additions & 0 deletions src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.cs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
// 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.

using System;
using System.Reflection;

public class TestSetValue
{
public static readonly long MagicNumber = 42;
}

public class TestSetValueDirect
{
public static readonly string MagicString = "";
}

class Test
{
public static int Main()
{
// Validate that the readonly static field cannot be set via reflection when the static constructor is triggered
// by the reflection SetValue operation itself.

try
{
typeof(TestSetValue).GetField(nameof(TestSetValue.MagicNumber)).SetValue(null, 0x123456789);
Console.WriteLine("FAILED: TestSetValue - Exception expected");
return -1;
}
catch (FieldAccessException)
{
Console.WriteLine("TestSetValue - Caught expected exception");
}

try
{
int i = 0;
typeof(TestSetValueDirect).GetField(nameof(TestSetValueDirect.MagicString)).SetValueDirect(__makeref(i), "Hello");
Console.WriteLine("FAILED: TestSetValueDirect - Exception expected");
return -1;
}
catch (FieldAccessException)
{
Console.WriteLine("TestSetValueDirect - Caught expected exception");
}
return 100;
}
}


9 changes: 9 additions & 0 deletions src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.csproj
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
</PropertyGroup>
<ItemGroup>
<Compile Include="TrySetReadonlyStaticField2.cs" />
</ItemGroup>
</Project>
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Fix detection of static readonly field re-initialization via reflection#37849

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.

 Sign up for GitHub

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

 Merged

jkotas
merged 3 commits into

from

Jun 15, 2020
 Merged


Fix detection of static readonly field re-initialization via reflection
#37849
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
5af8287

Fix detection of static readonly field re-initialization via reflection


jkotas Jun 12, 2020
2a580ac

Apply suggestions from code review


jkotas Jun 15, 2020
c055567

Fix test disable


jkotas Jun 15, 2020

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


58 changes: 31 additions & 27 deletions
src/coreclr/src/vm/invokeutil.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -941,11 +941,6 @@ void InvokeUtil::SetValidField(CorElementType fldType,
 }
 CONTRACTL_END;

 // We don't allow setting the field of nullable<T> (hasValue and value)
 // Because you can't independantly set them for this type.
 if (!declaringType.IsNull() && Nullable::IsNullableType(declaringType.GetMethodTable()))
 COMPlusThrow(kNotSupportedException);

 // call the <cinit>
 OBJECTREF Throwable = NULL;

Expand All@@ -954,27 +949,28 @@ void InvokeUtil::SetValidField(CorElementType fldType,
 {
 pDeclMT = declaringType.GetMethodTable();

 // We don't allow setting the field of nullable<T> (hasValue and value)
 // Because you can't independently set them for this type.
 if (Nullable::IsNullableType(pDeclMT))
 COMPlusThrow(kNotSupportedException);

 if (pDeclMT->IsSharedByGenericInstantiations())
 COMPlusThrow(kNotSupportedException, W("NotSupported_Type"));
 }
 else
 {
 pDeclMT = pField->GetModule()->GetGlobalMethodTable();
 }

 if (*pDomainInitialized == FALSE)
 {
 EX_TRY
 {
 if (declaringType.IsNull())
 {
 pField->GetModule()->GetGlobalMethodTable()->EnsureInstanceActive();
 pField->GetModule()->GetGlobalMethodTable()->CheckRunClassInitThrowing();
 }
 else
 {
 pDeclMT->EnsureInstanceActive();
 pDeclMT->CheckRunClassInitThrowing();

 *pDomainInitialized = TRUE;
 }
 }
 EX_CATCH_THROWABLE(&Throwable);
 }
#ifdef _DEBUG
Expand All@@ -990,6 +986,18 @@ void InvokeUtil::SetValidField(CorElementType fldType,
 GCPROTECT_END();
 }

 // Verify we're not trying to set the value of a static initonly field
 // once the class has been initialized.
 if (pField->IsStatic() && pDeclMT->IsClassInited() && IsFdInitOnly(pField->GetAttributes()))
 {
 DefineFullyQualifiedNameForClassW();
 SString ssFieldName(SString::Utf8, pField->GetName());
 COMPlusThrow(kFieldAccessException,
 IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
 ssFieldName.GetUnicode(),
 GetFullyQualifiedNameForClassW(pDeclMT));
 }

 // Set the field
 ARG_SLOT value;

Expand DownExpand Up@@ -1162,27 +1170,29 @@ OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJ
 {
 pDeclMT = declaringType.GetMethodTable();

 // We don't allow getting the field just so we don't have more specical
 // cases than we need to. Then we need at least the throw check to ensure
 // we don't allow data corruption.
 if (Nullable::IsNullableType(pDeclMT))
 COMPlusThrow(kNotSupportedException);

 if (pDeclMT->IsSharedByGenericInstantiations())
 COMPlusThrow(kNotSupportedException, W("NotSupported_Type"));
 }
 else
 {
 pDeclMT = pField->GetModule()->GetGlobalMethodTable();
 }

 if (*pDomainInitialized == FALSE)
 {
 EX_TRY
 {
 if (declaringType.IsNull())
 {
 pField->GetModule()->GetGlobalMethodTable()->EnsureInstanceActive();
 pField->GetModule()->GetGlobalMethodTable()->CheckRunClassInitThrowing();
 }
 else
 {
 pDeclMT->EnsureInstanceActive();
 pDeclMT->CheckRunClassInitThrowing();

 *pDomainInitialized = TRUE;
 }
 }
 EX_CATCH_THROWABLE(&Throwable);
 }
#ifdef _DEBUG
Expand All@@ -1199,12 +1209,6 @@ OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJ
 GCPROTECT_END();
 }

 // We don't allow getting the field just so we don't have more specical
 // cases than we need to. The we need at least the throw check to insure
 // we don't allow data corruption, but
 if (!declaringType.IsNull() && Nullable::IsNullableType(pDeclMT))
 COMPlusThrow(kNotSupportedException);

 CorElementType fieldElementType = pField->GetFieldType();

 switch (fieldElementType) {
Expand Down

36 changes: 2 additions & 34 deletions
src/coreclr/src/vm/reflectioninvocation.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -328,23 +328,6 @@ FCIMPL7(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Ob

 HELPER_METHOD_FRAME_BEGIN_PROTECT(gc);

 // Verify we're not trying to set the value of a static initonly field
 // once the class has been initialized.
 if (pFieldDesc->IsStatic())
 {
 MethodTable* pEnclosingMT = pFieldDesc->GetEnclosingMethodTable();
 if (pEnclosingMT->IsClassInited() && IsFdInitOnly(pFieldDesc->GetAttributes()))
 {
 DefineFullyQualifiedNameForClassW();
 SString ssFieldName(SString::Utf8, pFieldDesc->GetName());
 COMPlusThrow(kFieldAccessException,
 IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
 ssFieldName.GetUnicode(),
 GetFullyQualifiedNameForClassW(pEnclosingMT));
 }
 }

 //TODO: cleanup this function
 InvokeUtil::SetValidField(fieldType.GetSignatureCorElementType(), fieldType, pFieldDesc, &gc.target, &gc.value, declaringType, pDomainInitialized);

 HELPER_METHOD_FRAME_END();
Expand DownExpand Up@@ -1582,7 +1565,6 @@ static void DirectObjectFieldSet(FieldDesc *pField, TypeHandle fieldType, TypeHa
 // Validate the target/fld type relationship
 InvokeUtil::ValidateObjectTarget(pField, enclosingType, &objref);

 InvokeUtil::ValidField(fieldType, pValue);
 InvokeUtil::SetValidField(pField->GetFieldType(), fieldType, pField, &objref, pValue, enclosingType, pDomainInitialized);
 GCPROTECT_END();
}
Expand DownExpand Up@@ -1626,22 +1608,8 @@ FCIMPL5(void, RuntimeFieldHandle::SetValueDirect, ReflectFieldObject *pFieldUNSA
 TypeHandle targetType = pTarget->type;
 MethodTable *pEnclosingMT = contextType.GetMethodTable();

 {
 // Verify that the value passed can be widened into the target
 InvokeUtil::ValidField(fieldType, &gc.oValue);

 // Verify we're not trying to set the value of a static initonly field
 // once the class has been initialized.
 if (pField->IsStatic() && pEnclosingMT->IsClassInited() && IsFdInitOnly(pField->GetAttributes()))
 {
 DefineFullyQualifiedNameForClassW();
 SString ssFieldName(SString::Utf8, pField->GetName());
 COMPlusThrow(kFieldAccessException,
 IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
 ssFieldName.GetUnicode(),
 GetFullyQualifiedNameForClassW(pEnclosingMT));
 }
 }
 // Verify that the value passed can be widened into the target
 InvokeUtil::ValidField(fieldType, &gc.oValue);

 CLR_BOOL domainInitialized = FALSE;
 if (pField->IsStatic() || !targetType.IsValueType()) {
Expand Down

3 changes: 3 additions & 0 deletions
src/coreclr/tests/issues.targets

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1571,6 +1571,9 @@
 <ExcludeList Include="$(XunitTestBinBase)/reflection/SetValue/TrySetReadonlyStaticField/**">
 <Issue>needs triage</Issue>
 </ExcludeList>
 <ExcludeList Include="$(XunitTestBinBase)/reflection/SetValue/TrySetReadonlyStaticField2/**">
 <Issue>https://github.com/dotnet/runtime/issues/37899</Issue>
 </ExcludeList>
 <ExcludeList Include="$(XunitTestBinBase)/Regressions/coreclr/15241/genericcontext/**">
 <Issue>needs triage</Issue>
 </ExcludeList>
Expand Down

51 changes: 51 additions & 0 deletions
src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.cs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
// 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.

using System;
using System.Reflection;

public class TestSetValue
{
 public static readonly long MagicNumber = 42;
}

public class TestSetValueDirect
{
 public static readonly string MagicString = "";
}

class Test
{
 public static int Main()
 {
 // Validate that the readonly static field cannot be set via reflection when the static constructor is triggered 
 // by the reflection SetValue operation itself.

 try
 {
 typeof(TestSetValue).GetField(nameof(TestSetValue.MagicNumber)).SetValue(null, 0x123456789);
 Console.WriteLine("FAILED: TestSetValue - Exception expected");
 return -1;
 }
 catch (FieldAccessException)
 {
 Console.WriteLine("TestSetValue - Caught expected exception");
 }

 try 
 {
 int i = 0;
 typeof(TestSetValueDirect).GetField(nameof(TestSetValueDirect.MagicString)).SetValueDirect(__makeref(i), "Hello");
 Console.WriteLine("FAILED: TestSetValueDirect - Exception expected");
 return -1;
 }
 catch (FieldAccessException)
 {
 Console.WriteLine("TestSetValueDirect - Caught expected exception");
 }
 return 100;
 }
}



9 changes: 9 additions & 0 deletions
src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.csproj

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">
 <PropertyGroup>
 <OutputType>Exe</OutputType>
 <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
 </PropertyGroup>
 <ItemGroup>
 <Compile Include="TrySetReadonlyStaticField2.cs" />
 </ItemGroup>
</Project>

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Fix detection of static readonly field re-initialization via reflection#37849

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.

 Sign up for GitHub

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

 Merged

jkotas
merged 3 commits into

from

Jun 15, 2020
 Merged


Fix detection of static readonly field re-initialization via reflection
#37849
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
5af8287

Fix detection of static readonly field re-initialization via reflection


jkotas Jun 12, 2020
2a580ac

Apply suggestions from code review


jkotas Jun 15, 2020
c055567

Fix test disable


jkotas Jun 15, 2020

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


58 changes: 31 additions & 27 deletions
src/coreclr/src/vm/invokeutil.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -941,11 +941,6 @@ void InvokeUtil::SetValidField(CorElementType fldType,
 }
 CONTRACTL_END;

 // We don't allow setting the field of nullable<T> (hasValue and value)
 // Because you can't independantly set them for this type.
 if (!declaringType.IsNull() && Nullable::IsNullableType(declaringType.GetMethodTable()))
 COMPlusThrow(kNotSupportedException);

 // call the <cinit>
 OBJECTREF Throwable = NULL;

Expand All@@ -954,27 +949,28 @@ void InvokeUtil::SetValidField(CorElementType fldType,
 {
 pDeclMT = declaringType.GetMethodTable();

 // We don't allow setting the field of nullable<T> (hasValue and value)
 // Because you can't independently set them for this type.
 if (Nullable::IsNullableType(pDeclMT))
 COMPlusThrow(kNotSupportedException);

 if (pDeclMT->IsSharedByGenericInstantiations())
 COMPlusThrow(kNotSupportedException, W("NotSupported_Type"));
 }
 else
 {
 pDeclMT = pField->GetModule()->GetGlobalMethodTable();
 }

 if (*pDomainInitialized == FALSE)
 {
 EX_TRY
 {
 if (declaringType.IsNull())
 {
 pField->GetModule()->GetGlobalMethodTable()->EnsureInstanceActive();
 pField->GetModule()->GetGlobalMethodTable()->CheckRunClassInitThrowing();
 }
 else
 {
 pDeclMT->EnsureInstanceActive();
 pDeclMT->CheckRunClassInitThrowing();

 *pDomainInitialized = TRUE;
 }
 }
 EX_CATCH_THROWABLE(&Throwable);
 }
#ifdef _DEBUG
Expand All@@ -990,6 +986,18 @@ void InvokeUtil::SetValidField(CorElementType fldType,
 GCPROTECT_END();
 }

 // Verify we're not trying to set the value of a static initonly field
 // once the class has been initialized.
 if (pField->IsStatic() && pDeclMT->IsClassInited() && IsFdInitOnly(pField->GetAttributes()))
 {
 DefineFullyQualifiedNameForClassW();
 SString ssFieldName(SString::Utf8, pField->GetName());
 COMPlusThrow(kFieldAccessException,
 IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
 ssFieldName.GetUnicode(),
 GetFullyQualifiedNameForClassW(pDeclMT));
 }

 // Set the field
 ARG_SLOT value;

Expand DownExpand Up@@ -1162,27 +1170,29 @@ OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJ
 {
 pDeclMT = declaringType.GetMethodTable();

 // We don't allow getting the field just so we don't have more specical
 // cases than we need to. Then we need at least the throw check to ensure
 // we don't allow data corruption.
 if (Nullable::IsNullableType(pDeclMT))
 COMPlusThrow(kNotSupportedException);

 if (pDeclMT->IsSharedByGenericInstantiations())
 COMPlusThrow(kNotSupportedException, W("NotSupported_Type"));
 }
 else
 {
 pDeclMT = pField->GetModule()->GetGlobalMethodTable();
 }

 if (*pDomainInitialized == FALSE)
 {
 EX_TRY
 {
 if (declaringType.IsNull())
 {
 pField->GetModule()->GetGlobalMethodTable()->EnsureInstanceActive();
 pField->GetModule()->GetGlobalMethodTable()->CheckRunClassInitThrowing();
 }
 else
 {
 pDeclMT->EnsureInstanceActive();
 pDeclMT->CheckRunClassInitThrowing();

 *pDomainInitialized = TRUE;
 }
 }
 EX_CATCH_THROWABLE(&Throwable);
 }
#ifdef _DEBUG
Expand All@@ -1199,12 +1209,6 @@ OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJ
 GCPROTECT_END();
 }

 // We don't allow getting the field just so we don't have more specical
 // cases than we need to. The we need at least the throw check to insure
 // we don't allow data corruption, but
 if (!declaringType.IsNull() && Nullable::IsNullableType(pDeclMT))
 COMPlusThrow(kNotSupportedException);

 CorElementType fieldElementType = pField->GetFieldType();

 switch (fieldElementType) {
Expand Down

36 changes: 2 additions & 34 deletions
src/coreclr/src/vm/reflectioninvocation.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -328,23 +328,6 @@ FCIMPL7(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Ob

 HELPER_METHOD_FRAME_BEGIN_PROTECT(gc);

 // Verify we're not trying to set the value of a static initonly field
 // once the class has been initialized.
 if (pFieldDesc->IsStatic())
 {
 MethodTable* pEnclosingMT = pFieldDesc->GetEnclosingMethodTable();
 if (pEnclosingMT->IsClassInited() && IsFdInitOnly(pFieldDesc->GetAttributes()))
 {
 DefineFullyQualifiedNameForClassW();
 SString ssFieldName(SString::Utf8, pFieldDesc->GetName());
 COMPlusThrow(kFieldAccessException,
 IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
 ssFieldName.GetUnicode(),
 GetFullyQualifiedNameForClassW(pEnclosingMT));
 }
 }

 //TODO: cleanup this function
 InvokeUtil::SetValidField(fieldType.GetSignatureCorElementType(), fieldType, pFieldDesc, &gc.target, &gc.value, declaringType, pDomainInitialized);

 HELPER_METHOD_FRAME_END();
Expand DownExpand Up@@ -1582,7 +1565,6 @@ static void DirectObjectFieldSet(FieldDesc *pField, TypeHandle fieldType, TypeHa
 // Validate the target/fld type relationship
 InvokeUtil::ValidateObjectTarget(pField, enclosingType, &objref);

 InvokeUtil::ValidField(fieldType, pValue);
 InvokeUtil::SetValidField(pField->GetFieldType(), fieldType, pField, &objref, pValue, enclosingType, pDomainInitialized);
 GCPROTECT_END();
}
Expand DownExpand Up@@ -1626,22 +1608,8 @@ FCIMPL5(void, RuntimeFieldHandle::SetValueDirect, ReflectFieldObject *pFieldUNSA
 TypeHandle targetType = pTarget->type;
 MethodTable *pEnclosingMT = contextType.GetMethodTable();

 {
 // Verify that the value passed can be widened into the target
 InvokeUtil::ValidField(fieldType, &gc.oValue);

 // Verify we're not trying to set the value of a static initonly field
 // once the class has been initialized.
 if (pField->IsStatic() && pEnclosingMT->IsClassInited() && IsFdInitOnly(pField->GetAttributes()))
 {
 DefineFullyQualifiedNameForClassW();
 SString ssFieldName(SString::Utf8, pField->GetName());
 COMPlusThrow(kFieldAccessException,
 IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
 ssFieldName.GetUnicode(),
 GetFullyQualifiedNameForClassW(pEnclosingMT));
 }
 }
 // Verify that the value passed can be widened into the target
 InvokeUtil::ValidField(fieldType, &gc.oValue);

 CLR_BOOL domainInitialized = FALSE;
 if (pField->IsStatic() || !targetType.IsValueType()) {
Expand Down

3 changes: 3 additions & 0 deletions
src/coreclr/tests/issues.targets

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1571,6 +1571,9 @@
 <ExcludeList Include="$(XunitTestBinBase)/reflection/SetValue/TrySetReadonlyStaticField/**">
 <Issue>needs triage</Issue>
 </ExcludeList>
 <ExcludeList Include="$(XunitTestBinBase)/reflection/SetValue/TrySetReadonlyStaticField2/**">
 <Issue>https://github.com/dotnet/runtime/issues/37899</Issue>
 </ExcludeList>
 <ExcludeList Include="$(XunitTestBinBase)/Regressions/coreclr/15241/genericcontext/**">
 <Issue>needs triage</Issue>
 </ExcludeList>
Expand Down

51 changes: 51 additions & 0 deletions
src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.cs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
// 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.

using System;
using System.Reflection;

public class TestSetValue
{
 public static readonly long MagicNumber = 42;
}

public class TestSetValueDirect
{
 public static readonly string MagicString = "";
}

class Test
{
 public static int Main()
 {
 // Validate that the readonly static field cannot be set via reflection when the static constructor is triggered 
 // by the reflection SetValue operation itself.

 try
 {
 typeof(TestSetValue).GetField(nameof(TestSetValue.MagicNumber)).SetValue(null, 0x123456789);
 Console.WriteLine("FAILED: TestSetValue - Exception expected");
 return -1;
 }
 catch (FieldAccessException)
 {
 Console.WriteLine("TestSetValue - Caught expected exception");
 }

 try 
 {
 int i = 0;
 typeof(TestSetValueDirect).GetField(nameof(TestSetValueDirect.MagicString)).SetValueDirect(__makeref(i), "Hello");
 Console.WriteLine("FAILED: TestSetValueDirect - Exception expected");
 return -1;
 }
 catch (FieldAccessException)
 {
 Console.WriteLine("TestSetValueDirect - Caught expected exception");
 }
 return 100;
 }
}



9 changes: 9 additions & 0 deletions
src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.csproj

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">
 <PropertyGroup>
 <OutputType>Exe</OutputType>
 <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
 </PropertyGroup>
 <ItemGroup>
 <Compile Include="TrySetReadonlyStaticField2.cs" />
 </ItemGroup>
</Project>

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Fix detection of static readonly field re-initialization via reflection#37849

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.

 Sign up for GitHub

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

 Merged

jkotas
merged 3 commits into

from

Jun 15, 2020
 Merged


Fix detection of static readonly field re-initialization via reflection
#37849
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
5af8287

Fix detection of static readonly field re-initialization via reflection


jkotas Jun 12, 2020
2a580ac

Apply suggestions from code review


jkotas Jun 15, 2020
c055567

Fix test disable


jkotas Jun 15, 2020

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


58 changes: 31 additions & 27 deletions
src/coreclr/src/vm/invokeutil.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -941,11 +941,6 @@ void InvokeUtil::SetValidField(CorElementType fldType,
 }
 CONTRACTL_END;

 // We don't allow setting the field of nullable<T> (hasValue and value)
 // Because you can't independantly set them for this type.
 if (!declaringType.IsNull() && Nullable::IsNullableType(declaringType.GetMethodTable()))
 COMPlusThrow(kNotSupportedException);

 // call the <cinit>
 OBJECTREF Throwable = NULL;

Expand All@@ -954,27 +949,28 @@ void InvokeUtil::SetValidField(CorElementType fldType,
 {
 pDeclMT = declaringType.GetMethodTable();

 // We don't allow setting the field of nullable<T> (hasValue and value)
 // Because you can't independently set them for this type.
 if (Nullable::IsNullableType(pDeclMT))
 COMPlusThrow(kNotSupportedException);

 if (pDeclMT->IsSharedByGenericInstantiations())
 COMPlusThrow(kNotSupportedException, W("NotSupported_Type"));
 }
 else
 {
 pDeclMT = pField->GetModule()->GetGlobalMethodTable();
 }

 if (*pDomainInitialized == FALSE)
 {
 EX_TRY
 {
 if (declaringType.IsNull())
 {
 pField->GetModule()->GetGlobalMethodTable()->EnsureInstanceActive();
 pField->GetModule()->GetGlobalMethodTable()->CheckRunClassInitThrowing();
 }
 else
 {
 pDeclMT->EnsureInstanceActive();
 pDeclMT->CheckRunClassInitThrowing();

 *pDomainInitialized = TRUE;
 }
 }
 EX_CATCH_THROWABLE(&Throwable);
 }
#ifdef _DEBUG
Expand All@@ -990,6 +986,18 @@ void InvokeUtil::SetValidField(CorElementType fldType,
 GCPROTECT_END();
 }

 // Verify we're not trying to set the value of a static initonly field
 // once the class has been initialized.
 if (pField->IsStatic() && pDeclMT->IsClassInited() && IsFdInitOnly(pField->GetAttributes()))
 {
 DefineFullyQualifiedNameForClassW();
 SString ssFieldName(SString::Utf8, pField->GetName());
 COMPlusThrow(kFieldAccessException,
 IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
 ssFieldName.GetUnicode(),
 GetFullyQualifiedNameForClassW(pDeclMT));
 }

 // Set the field
 ARG_SLOT value;

Expand DownExpand Up@@ -1162,27 +1170,29 @@ OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJ
 {
 pDeclMT = declaringType.GetMethodTable();

 // We don't allow getting the field just so we don't have more specical
 // cases than we need to. Then we need at least the throw check to ensure
 // we don't allow data corruption.
 if (Nullable::IsNullableType(pDeclMT))
 COMPlusThrow(kNotSupportedException);

 if (pDeclMT->IsSharedByGenericInstantiations())
 COMPlusThrow(kNotSupportedException, W("NotSupported_Type"));
 }
 else
 {
 pDeclMT = pField->GetModule()->GetGlobalMethodTable();
 }

 if (*pDomainInitialized == FALSE)
 {
 EX_TRY
 {
 if (declaringType.IsNull())
 {
 pField->GetModule()->GetGlobalMethodTable()->EnsureInstanceActive();
 pField->GetModule()->GetGlobalMethodTable()->CheckRunClassInitThrowing();
 }
 else
 {
 pDeclMT->EnsureInstanceActive();
 pDeclMT->CheckRunClassInitThrowing();

 *pDomainInitialized = TRUE;
 }
 }
 EX_CATCH_THROWABLE(&Throwable);
 }
#ifdef _DEBUG
Expand All@@ -1199,12 +1209,6 @@ OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJ
 GCPROTECT_END();
 }

 // We don't allow getting the field just so we don't have more specical
 // cases than we need to. The we need at least the throw check to insure
 // we don't allow data corruption, but
 if (!declaringType.IsNull() && Nullable::IsNullableType(pDeclMT))
 COMPlusThrow(kNotSupportedException);

 CorElementType fieldElementType = pField->GetFieldType();

 switch (fieldElementType) {
Expand Down

36 changes: 2 additions & 34 deletions
src/coreclr/src/vm/reflectioninvocation.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -328,23 +328,6 @@ FCIMPL7(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Ob

 HELPER_METHOD_FRAME_BEGIN_PROTECT(gc);

 // Verify we're not trying to set the value of a static initonly field
 // once the class has been initialized.
 if (pFieldDesc->IsStatic())
 {
 MethodTable* pEnclosingMT = pFieldDesc->GetEnclosingMethodTable();
 if (pEnclosingMT->IsClassInited() && IsFdInitOnly(pFieldDesc->GetAttributes()))
 {
 DefineFullyQualifiedNameForClassW();
 SString ssFieldName(SString::Utf8, pFieldDesc->GetName());
 COMPlusThrow(kFieldAccessException,
 IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
 ssFieldName.GetUnicode(),
 GetFullyQualifiedNameForClassW(pEnclosingMT));
 }
 }

 //TODO: cleanup this function
 InvokeUtil::SetValidField(fieldType.GetSignatureCorElementType(), fieldType, pFieldDesc, &gc.target, &gc.value, declaringType, pDomainInitialized);

 HELPER_METHOD_FRAME_END();
Expand DownExpand Up@@ -1582,7 +1565,6 @@ static void DirectObjectFieldSet(FieldDesc *pField, TypeHandle fieldType, TypeHa
 // Validate the target/fld type relationship
 InvokeUtil::ValidateObjectTarget(pField, enclosingType, &objref);

 InvokeUtil::ValidField(fieldType, pValue);
 InvokeUtil::SetValidField(pField->GetFieldType(), fieldType, pField, &objref, pValue, enclosingType, pDomainInitialized);
 GCPROTECT_END();
}
Expand DownExpand Up@@ -1626,22 +1608,8 @@ FCIMPL5(void, RuntimeFieldHandle::SetValueDirect, ReflectFieldObject *pFieldUNSA
 TypeHandle targetType = pTarget->type;
 MethodTable *pEnclosingMT = contextType.GetMethodTable();

 {
 // Verify that the value passed can be widened into the target
 InvokeUtil::ValidField(fieldType, &gc.oValue);

 // Verify we're not trying to set the value of a static initonly field
 // once the class has been initialized.
 if (pField->IsStatic() && pEnclosingMT->IsClassInited() && IsFdInitOnly(pField->GetAttributes()))
 {
 DefineFullyQualifiedNameForClassW();
 SString ssFieldName(SString::Utf8, pField->GetName());
 COMPlusThrow(kFieldAccessException,
 IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
 ssFieldName.GetUnicode(),
 GetFullyQualifiedNameForClassW(pEnclosingMT));
 }
 }
 // Verify that the value passed can be widened into the target
 InvokeUtil::ValidField(fieldType, &gc.oValue);

 CLR_BOOL domainInitialized = FALSE;
 if (pField->IsStatic() || !targetType.IsValueType()) {
Expand Down

3 changes: 3 additions & 0 deletions
src/coreclr/tests/issues.targets

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1571,6 +1571,9 @@
 <ExcludeList Include="$(XunitTestBinBase)/reflection/SetValue/TrySetReadonlyStaticField/**">
 <Issue>needs triage</Issue>
 </ExcludeList>
 <ExcludeList Include="$(XunitTestBinBase)/reflection/SetValue/TrySetReadonlyStaticField2/**">
 <Issue>https://github.com/dotnet/runtime/issues/37899</Issue>
 </ExcludeList>
 <ExcludeList Include="$(XunitTestBinBase)/Regressions/coreclr/15241/genericcontext/**">
 <Issue>needs triage</Issue>
 </ExcludeList>
Expand Down

51 changes: 51 additions & 0 deletions
src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.cs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
// 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.

using System;
using System.Reflection;

public class TestSetValue
{
 public static readonly long MagicNumber = 42;
}

public class TestSetValueDirect
{
 public static readonly string MagicString = "";
}

class Test
{
 public static int Main()
 {
 // Validate that the readonly static field cannot be set via reflection when the static constructor is triggered 
 // by the reflection SetValue operation itself.

 try
 {
 typeof(TestSetValue).GetField(nameof(TestSetValue.MagicNumber)).SetValue(null, 0x123456789);
 Console.WriteLine("FAILED: TestSetValue - Exception expected");
 return -1;
 }
 catch (FieldAccessException)
 {
 Console.WriteLine("TestSetValue - Caught expected exception");
 }

 try 
 {
 int i = 0;
 typeof(TestSetValueDirect).GetField(nameof(TestSetValueDirect.MagicString)).SetValueDirect(__makeref(i), "Hello");
 Console.WriteLine("FAILED: TestSetValueDirect - Exception expected");
 return -1;
 }
 catch (FieldAccessException)
 {
 Console.WriteLine("TestSetValueDirect - Caught expected exception");
 }
 return 100;
 }
}



9 changes: 9 additions & 0 deletions
src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.csproj

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">
 <PropertyGroup>
 <OutputType>Exe</OutputType>
 <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
 </PropertyGroup>
 <ItemGroup>
 <Compile Include="TrySetReadonlyStaticField2.cs" />
 </ItemGroup>
</Project>

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Fix detection of static readonly field re-initialization via reflection#37849

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.

 Sign up for GitHub

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

 Merged

jkotas
merged 3 commits into

from

Jun 15, 2020
 Merged


Fix detection of static readonly field re-initialization via reflection
#37849
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
5af8287

Fix detection of static readonly field re-initialization via reflection


jkotas Jun 12, 2020
2a580ac

Apply suggestions from code review


jkotas Jun 15, 2020
c055567

Fix test disable


jkotas Jun 15, 2020

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


58 changes: 31 additions & 27 deletions
src/coreclr/src/vm/invokeutil.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -941,11 +941,6 @@ void InvokeUtil::SetValidField(CorElementType fldType,
 }
 CONTRACTL_END;

 // We don't allow setting the field of nullable<T> (hasValue and value)
 // Because you can't independantly set them for this type.
 if (!declaringType.IsNull() && Nullable::IsNullableType(declaringType.GetMethodTable()))
 COMPlusThrow(kNotSupportedException);

 // call the <cinit>
 OBJECTREF Throwable = NULL;

Expand All@@ -954,27 +949,28 @@ void InvokeUtil::SetValidField(CorElementType fldType,
 {
 pDeclMT = declaringType.GetMethodTable();

 // We don't allow setting the field of nullable<T> (hasValue and value)
 // Because you can't independently set them for this type.
 if (Nullable::IsNullableType(pDeclMT))
 COMPlusThrow(kNotSupportedException);

 if (pDeclMT->IsSharedByGenericInstantiations())
 COMPlusThrow(kNotSupportedException, W("NotSupported_Type"));
 }
 else
 {
 pDeclMT = pField->GetModule()->GetGlobalMethodTable();
 }

 if (*pDomainInitialized == FALSE)
 {
 EX_TRY
 {
 if (declaringType.IsNull())
 {
 pField->GetModule()->GetGlobalMethodTable()->EnsureInstanceActive();
 pField->GetModule()->GetGlobalMethodTable()->CheckRunClassInitThrowing();
 }
 else
 {
 pDeclMT->EnsureInstanceActive();
 pDeclMT->CheckRunClassInitThrowing();

 *pDomainInitialized = TRUE;
 }
 }
 EX_CATCH_THROWABLE(&Throwable);
 }
#ifdef _DEBUG
Expand All@@ -990,6 +986,18 @@ void InvokeUtil::SetValidField(CorElementType fldType,
 GCPROTECT_END();
 }

 // Verify we're not trying to set the value of a static initonly field
 // once the class has been initialized.
 if (pField->IsStatic() && pDeclMT->IsClassInited() && IsFdInitOnly(pField->GetAttributes()))
 {
 DefineFullyQualifiedNameForClassW();
 SString ssFieldName(SString::Utf8, pField->GetName());
 COMPlusThrow(kFieldAccessException,
 IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
 ssFieldName.GetUnicode(),
 GetFullyQualifiedNameForClassW(pDeclMT));
 }

 // Set the field
 ARG_SLOT value;

Expand DownExpand Up@@ -1162,27 +1170,29 @@ OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJ
 {
 pDeclMT = declaringType.GetMethodTable();

 // We don't allow getting the field just so we don't have more specical
 // cases than we need to. Then we need at least the throw check to ensure
 // we don't allow data corruption.
 if (Nullable::IsNullableType(pDeclMT))
 COMPlusThrow(kNotSupportedException);

 if (pDeclMT->IsSharedByGenericInstantiations())
 COMPlusThrow(kNotSupportedException, W("NotSupported_Type"));
 }
 else
 {
 pDeclMT = pField->GetModule()->GetGlobalMethodTable();
 }

 if (*pDomainInitialized == FALSE)
 {
 EX_TRY
 {
 if (declaringType.IsNull())
 {
 pField->GetModule()->GetGlobalMethodTable()->EnsureInstanceActive();
 pField->GetModule()->GetGlobalMethodTable()->CheckRunClassInitThrowing();
 }
 else
 {
 pDeclMT->EnsureInstanceActive();
 pDeclMT->CheckRunClassInitThrowing();

 *pDomainInitialized = TRUE;
 }
 }
 EX_CATCH_THROWABLE(&Throwable);
 }
#ifdef _DEBUG
Expand All@@ -1199,12 +1209,6 @@ OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJ
 GCPROTECT_END();
 }

 // We don't allow getting the field just so we don't have more specical
 // cases than we need to. The we need at least the throw check to insure
 // we don't allow data corruption, but
 if (!declaringType.IsNull() && Nullable::IsNullableType(pDeclMT))
 COMPlusThrow(kNotSupportedException);

 CorElementType fieldElementType = pField->GetFieldType();

 switch (fieldElementType) {
Expand Down

36 changes: 2 additions & 34 deletions
src/coreclr/src/vm/reflectioninvocation.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -328,23 +328,6 @@ FCIMPL7(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Ob

 HELPER_METHOD_FRAME_BEGIN_PROTECT(gc);

 // Verify we're not trying to set the value of a static initonly field
 // once the class has been initialized.
 if (pFieldDesc->IsStatic())
 {
 MethodTable* pEnclosingMT = pFieldDesc->GetEnclosingMethodTable();
 if (pEnclosingMT->IsClassInited() && IsFdInitOnly(pFieldDesc->GetAttributes()))
 {
 DefineFullyQualifiedNameForClassW();
 SString ssFieldName(SString::Utf8, pFieldDesc->GetName());
 COMPlusThrow(kFieldAccessException,
 IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
 ssFieldName.GetUnicode(),
 GetFullyQualifiedNameForClassW(pEnclosingMT));
 }
 }

 //TODO: cleanup this function
 InvokeUtil::SetValidField(fieldType.GetSignatureCorElementType(), fieldType, pFieldDesc, &gc.target, &gc.value, declaringType, pDomainInitialized);

 HELPER_METHOD_FRAME_END();
Expand DownExpand Up@@ -1582,7 +1565,6 @@ static void DirectObjectFieldSet(FieldDesc *pField, TypeHandle fieldType, TypeHa
 // Validate the target/fld type relationship
 InvokeUtil::ValidateObjectTarget(pField, enclosingType, &objref);

 InvokeUtil::ValidField(fieldType, pValue);
 InvokeUtil::SetValidField(pField->GetFieldType(), fieldType, pField, &objref, pValue, enclosingType, pDomainInitialized);
 GCPROTECT_END();
}
Expand DownExpand Up@@ -1626,22 +1608,8 @@ FCIMPL5(void, RuntimeFieldHandle::SetValueDirect, ReflectFieldObject *pFieldUNSA
 TypeHandle targetType = pTarget->type;
 MethodTable *pEnclosingMT = contextType.GetMethodTable();

 {
 // Verify that the value passed can be widened into the target
 InvokeUtil::ValidField(fieldType, &gc.oValue);

 // Verify we're not trying to set the value of a static initonly field
 // once the class has been initialized.
 if (pField->IsStatic() && pEnclosingMT->IsClassInited() && IsFdInitOnly(pField->GetAttributes()))
 {
 DefineFullyQualifiedNameForClassW();
 SString ssFieldName(SString::Utf8, pField->GetName());
 COMPlusThrow(kFieldAccessException,
 IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
 ssFieldName.GetUnicode(),
 GetFullyQualifiedNameForClassW(pEnclosingMT));
 }
 }
 // Verify that the value passed can be widened into the target
 InvokeUtil::ValidField(fieldType, &gc.oValue);

 CLR_BOOL domainInitialized = FALSE;
 if (pField->IsStatic() || !targetType.IsValueType()) {
Expand Down

3 changes: 3 additions & 0 deletions
src/coreclr/tests/issues.targets

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1571,6 +1571,9 @@
 <ExcludeList Include="$(XunitTestBinBase)/reflection/SetValue/TrySetReadonlyStaticField/**">
 <Issue>needs triage</Issue>
 </ExcludeList>
 <ExcludeList Include="$(XunitTestBinBase)/reflection/SetValue/TrySetReadonlyStaticField2/**">
 <Issue>https://github.com/dotnet/runtime/issues/37899</Issue>
 </ExcludeList>
 <ExcludeList Include="$(XunitTestBinBase)/Regressions/coreclr/15241/genericcontext/**">
 <Issue>needs triage</Issue>
 </ExcludeList>
Expand Down

51 changes: 51 additions & 0 deletions
src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.cs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
// 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.

using System;
using System.Reflection;

public class TestSetValue
{
 public static readonly long MagicNumber = 42;
}

public class TestSetValueDirect
{
 public static readonly string MagicString = "";
}

class Test
{
 public static int Main()
 {
 // Validate that the readonly static field cannot be set via reflection when the static constructor is triggered 
 // by the reflection SetValue operation itself.

 try
 {
 typeof(TestSetValue).GetField(nameof(TestSetValue.MagicNumber)).SetValue(null, 0x123456789);
 Console.WriteLine("FAILED: TestSetValue - Exception expected");
 return -1;
 }
 catch (FieldAccessException)
 {
 Console.WriteLine("TestSetValue - Caught expected exception");
 }

 try 
 {
 int i = 0;
 typeof(TestSetValueDirect).GetField(nameof(TestSetValueDirect.MagicString)).SetValueDirect(__makeref(i), "Hello");
 Console.WriteLine("FAILED: TestSetValueDirect - Exception expected");
 return -1;
 }
 catch (FieldAccessException)
 {
 Console.WriteLine("TestSetValueDirect - Caught expected exception");
 }
 return 100;
 }
}



9 changes: 9 additions & 0 deletions
src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.csproj

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">
 <PropertyGroup>
 <OutputType>Exe</OutputType>
 <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
 </PropertyGroup>
 <ItemGroup>
 <Compile Include="TrySetReadonlyStaticField2.cs" />
 </ItemGroup>
</Project>

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Fix detection of static readonly field re-initialization via reflection#37849

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.

 Sign up for GitHub

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

 Merged

jkotas
merged 3 commits into

from

Jun 15, 2020
 Merged


Fix detection of static readonly field re-initialization via reflection
#37849
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
5af8287

Fix detection of static readonly field re-initialization via reflection


jkotas Jun 12, 2020
2a580ac

Apply suggestions from code review


jkotas Jun 15, 2020
c055567

Fix test disable


jkotas Jun 15, 2020

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


58 changes: 31 additions & 27 deletions
src/coreclr/src/vm/invokeutil.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -941,11 +941,6 @@ void InvokeUtil::SetValidField(CorElementType fldType,
 }
 CONTRACTL_END;

 // We don't allow setting the field of nullable<T> (hasValue and value)
 // Because you can't independantly set them for this type.
 if (!declaringType.IsNull() && Nullable::IsNullableType(declaringType.GetMethodTable()))
 COMPlusThrow(kNotSupportedException);

 // call the <cinit>
 OBJECTREF Throwable = NULL;

Expand All@@ -954,27 +949,28 @@ void InvokeUtil::SetValidField(CorElementType fldType,
 {
 pDeclMT = declaringType.GetMethodTable();

 // We don't allow setting the field of nullable<T> (hasValue and value)
 // Because you can't independently set them for this type.
 if (Nullable::IsNullableType(pDeclMT))
 COMPlusThrow(kNotSupportedException);

 if (pDeclMT->IsSharedByGenericInstantiations())
 COMPlusThrow(kNotSupportedException, W("NotSupported_Type"));
 }
 else
 {
 pDeclMT = pField->GetModule()->GetGlobalMethodTable();
 }

 if (*pDomainInitialized == FALSE)
 {
 EX_TRY
 {
 if (declaringType.IsNull())
 {
 pField->GetModule()->GetGlobalMethodTable()->EnsureInstanceActive();
 pField->GetModule()->GetGlobalMethodTable()->CheckRunClassInitThrowing();
 }
 else
 {
 pDeclMT->EnsureInstanceActive();
 pDeclMT->CheckRunClassInitThrowing();

 *pDomainInitialized = TRUE;
 }
 }
 EX_CATCH_THROWABLE(&Throwable);
 }
#ifdef _DEBUG
Expand All@@ -990,6 +986,18 @@ void InvokeUtil::SetValidField(CorElementType fldType,
 GCPROTECT_END();
 }

 // Verify we're not trying to set the value of a static initonly field
 // once the class has been initialized.
 if (pField->IsStatic() && pDeclMT->IsClassInited() && IsFdInitOnly(pField->GetAttributes()))
 {
 DefineFullyQualifiedNameForClassW();
 SString ssFieldName(SString::Utf8, pField->GetName());
 COMPlusThrow(kFieldAccessException,
 IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
 ssFieldName.GetUnicode(),
 GetFullyQualifiedNameForClassW(pDeclMT));
 }

 // Set the field
 ARG_SLOT value;

Expand DownExpand Up@@ -1162,27 +1170,29 @@ OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJ
 {
 pDeclMT = declaringType.GetMethodTable();

 // We don't allow getting the field just so we don't have more specical
 // cases than we need to. Then we need at least the throw check to ensure
 // we don't allow data corruption.
 if (Nullable::IsNullableType(pDeclMT))
 COMPlusThrow(kNotSupportedException);

 if (pDeclMT->IsSharedByGenericInstantiations())
 COMPlusThrow(kNotSupportedException, W("NotSupported_Type"));
 }
 else
 {
 pDeclMT = pField->GetModule()->GetGlobalMethodTable();
 }

 if (*pDomainInitialized == FALSE)
 {
 EX_TRY
 {
 if (declaringType.IsNull())
 {
 pField->GetModule()->GetGlobalMethodTable()->EnsureInstanceActive();
 pField->GetModule()->GetGlobalMethodTable()->CheckRunClassInitThrowing();
 }
 else
 {
 pDeclMT->EnsureInstanceActive();
 pDeclMT->CheckRunClassInitThrowing();

 *pDomainInitialized = TRUE;
 }
 }
 EX_CATCH_THROWABLE(&Throwable);
 }
#ifdef _DEBUG
Expand All@@ -1199,12 +1209,6 @@ OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJ
 GCPROTECT_END();
 }

 // We don't allow getting the field just so we don't have more specical
 // cases than we need to. The we need at least the throw check to insure
 // we don't allow data corruption, but
 if (!declaringType.IsNull() && Nullable::IsNullableType(pDeclMT))
 COMPlusThrow(kNotSupportedException);

 CorElementType fieldElementType = pField->GetFieldType();

 switch (fieldElementType) {
Expand Down

36 changes: 2 additions & 34 deletions
src/coreclr/src/vm/reflectioninvocation.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -328,23 +328,6 @@ FCIMPL7(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Ob

 HELPER_METHOD_FRAME_BEGIN_PROTECT(gc);

 // Verify we're not trying to set the value of a static initonly field
 // once the class has been initialized.
 if (pFieldDesc->IsStatic())
 {
 MethodTable* pEnclosingMT = pFieldDesc->GetEnclosingMethodTable();
 if (pEnclosingMT->IsClassInited() && IsFdInitOnly(pFieldDesc->GetAttributes()))
 {
 DefineFullyQualifiedNameForClassW();
 SString ssFieldName(SString::Utf8, pFieldDesc->GetName());
 COMPlusThrow(kFieldAccessException,
 IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
 ssFieldName.GetUnicode(),
 GetFullyQualifiedNameForClassW(pEnclosingMT));
 }
 }

 //TODO: cleanup this function
 InvokeUtil::SetValidField(fieldType.GetSignatureCorElementType(), fieldType, pFieldDesc, &gc.target, &gc.value, declaringType, pDomainInitialized);

 HELPER_METHOD_FRAME_END();
Expand DownExpand Up@@ -1582,7 +1565,6 @@ static void DirectObjectFieldSet(FieldDesc *pField, TypeHandle fieldType, TypeHa
 // Validate the target/fld type relationship
 InvokeUtil::ValidateObjectTarget(pField, enclosingType, &objref);

 InvokeUtil::ValidField(fieldType, pValue);
 InvokeUtil::SetValidField(pField->GetFieldType(), fieldType, pField, &objref, pValue, enclosingType, pDomainInitialized);
 GCPROTECT_END();
}
Expand DownExpand Up@@ -1626,22 +1608,8 @@ FCIMPL5(void, RuntimeFieldHandle::SetValueDirect, ReflectFieldObject *pFieldUNSA
 TypeHandle targetType = pTarget->type;
 MethodTable *pEnclosingMT = contextType.GetMethodTable();

 {
 // Verify that the value passed can be widened into the target
 InvokeUtil::ValidField(fieldType, &gc.oValue);

 // Verify we're not trying to set the value of a static initonly field
 // once the class has been initialized.
 if (pField->IsStatic() && pEnclosingMT->IsClassInited() && IsFdInitOnly(pField->GetAttributes()))
 {
 DefineFullyQualifiedNameForClassW();
 SString ssFieldName(SString::Utf8, pField->GetName());
 COMPlusThrow(kFieldAccessException,
 IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
 ssFieldName.GetUnicode(),
 GetFullyQualifiedNameForClassW(pEnclosingMT));
 }
 }
 // Verify that the value passed can be widened into the target
 InvokeUtil::ValidField(fieldType, &gc.oValue);

 CLR_BOOL domainInitialized = FALSE;
 if (pField->IsStatic() || !targetType.IsValueType()) {
Expand Down

3 changes: 3 additions & 0 deletions
src/coreclr/tests/issues.targets

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1571,6 +1571,9 @@
 <ExcludeList Include="$(XunitTestBinBase)/reflection/SetValue/TrySetReadonlyStaticField/**">
 <Issue>needs triage</Issue>
 </ExcludeList>
 <ExcludeList Include="$(XunitTestBinBase)/reflection/SetValue/TrySetReadonlyStaticField2/**">
 <Issue>https://github.com/dotnet/runtime/issues/37899</Issue>
 </ExcludeList>
 <ExcludeList Include="$(XunitTestBinBase)/Regressions/coreclr/15241/genericcontext/**">
 <Issue>needs triage</Issue>
 </ExcludeList>
Expand Down

51 changes: 51 additions & 0 deletions
src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.cs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
// 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.

using System;
using System.Reflection;

public class TestSetValue
{
 public static readonly long MagicNumber = 42;
}

public class TestSetValueDirect
{
 public static readonly string MagicString = "";
}

class Test
{
 public static int Main()
 {
 // Validate that the readonly static field cannot be set via reflection when the static constructor is triggered 
 // by the reflection SetValue operation itself.

 try
 {
 typeof(TestSetValue).GetField(nameof(TestSetValue.MagicNumber)).SetValue(null, 0x123456789);
 Console.WriteLine("FAILED: TestSetValue - Exception expected");
 return -1;
 }
 catch (FieldAccessException)
 {
 Console.WriteLine("TestSetValue - Caught expected exception");
 }

 try 
 {
 int i = 0;
 typeof(TestSetValueDirect).GetField(nameof(TestSetValueDirect.MagicString)).SetValueDirect(__makeref(i), "Hello");
 Console.WriteLine("FAILED: TestSetValueDirect - Exception expected");
 return -1;
 }
 catch (FieldAccessException)
 {
 Console.WriteLine("TestSetValueDirect - Caught expected exception");
 }
 return 100;
 }
}



9 changes: 9 additions & 0 deletions
src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.csproj

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">
 <PropertyGroup>
 <OutputType>Exe</OutputType>
 <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
 </PropertyGroup>
 <ItemGroup>
 <Compile Include="TrySetReadonlyStaticField2.cs" />
 </ItemGroup>
</Project>

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Fix detection of static readonly field re-initialization via reflection#37849

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.

 Sign up for GitHub

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

 Merged

jkotas
merged 3 commits into

from

Jun 15, 2020
 Merged


Fix detection of static readonly field re-initialization via reflection
#37849
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
5af8287

Fix detection of static readonly field re-initialization via reflection


jkotas Jun 12, 2020
2a580ac

Apply suggestions from code review


jkotas Jun 15, 2020
c055567

Fix test disable


jkotas Jun 15, 2020

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


58 changes: 31 additions & 27 deletions
src/coreclr/src/vm/invokeutil.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -941,11 +941,6 @@ void InvokeUtil::SetValidField(CorElementType fldType,
 }
 CONTRACTL_END;

 // We don't allow setting the field of nullable<T> (hasValue and value)
 // Because you can't independantly set them for this type.
 if (!declaringType.IsNull() && Nullable::IsNullableType(declaringType.GetMethodTable()))
 COMPlusThrow(kNotSupportedException);

 // call the <cinit>
 OBJECTREF Throwable = NULL;

Expand All@@ -954,27 +949,28 @@ void InvokeUtil::SetValidField(CorElementType fldType,
 {
 pDeclMT = declaringType.GetMethodTable();

 // We don't allow setting the field of nullable<T> (hasValue and value)
 // Because you can't independently set them for this type.
 if (Nullable::IsNullableType(pDeclMT))
 COMPlusThrow(kNotSupportedException);

 if (pDeclMT->IsSharedByGenericInstantiations())
 COMPlusThrow(kNotSupportedException, W("NotSupported_Type"));
 }
 else
 {
 pDeclMT = pField->GetModule()->GetGlobalMethodTable();
 }

 if (*pDomainInitialized == FALSE)
 {
 EX_TRY
 {
 if (declaringType.IsNull())
 {
 pField->GetModule()->GetGlobalMethodTable()->EnsureInstanceActive();
 pField->GetModule()->GetGlobalMethodTable()->CheckRunClassInitThrowing();
 }
 else
 {
 pDeclMT->EnsureInstanceActive();
 pDeclMT->CheckRunClassInitThrowing();

 *pDomainInitialized = TRUE;
 }
 }
 EX_CATCH_THROWABLE(&Throwable);
 }
#ifdef _DEBUG
Expand All@@ -990,6 +986,18 @@ void InvokeUtil::SetValidField(CorElementType fldType,
 GCPROTECT_END();
 }

 // Verify we're not trying to set the value of a static initonly field
 // once the class has been initialized.
 if (pField->IsStatic() && pDeclMT->IsClassInited() && IsFdInitOnly(pField->GetAttributes()))
 {
 DefineFullyQualifiedNameForClassW();
 SString ssFieldName(SString::Utf8, pField->GetName());
 COMPlusThrow(kFieldAccessException,
 IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
 ssFieldName.GetUnicode(),
 GetFullyQualifiedNameForClassW(pDeclMT));
 }

 // Set the field
 ARG_SLOT value;

Expand DownExpand Up@@ -1162,27 +1170,29 @@ OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJ
 {
 pDeclMT = declaringType.GetMethodTable();

 // We don't allow getting the field just so we don't have more specical
 // cases than we need to. Then we need at least the throw check to ensure
 // we don't allow data corruption.
 if (Nullable::IsNullableType(pDeclMT))
 COMPlusThrow(kNotSupportedException);

 if (pDeclMT->IsSharedByGenericInstantiations())
 COMPlusThrow(kNotSupportedException, W("NotSupported_Type"));
 }
 else
 {
 pDeclMT = pField->GetModule()->GetGlobalMethodTable();
 }

 if (*pDomainInitialized == FALSE)
 {
 EX_TRY
 {
 if (declaringType.IsNull())
 {
 pField->GetModule()->GetGlobalMethodTable()->EnsureInstanceActive();
 pField->GetModule()->GetGlobalMethodTable()->CheckRunClassInitThrowing();
 }
 else
 {
 pDeclMT->EnsureInstanceActive();
 pDeclMT->CheckRunClassInitThrowing();

 *pDomainInitialized = TRUE;
 }
 }
 EX_CATCH_THROWABLE(&Throwable);
 }
#ifdef _DEBUG
Expand All@@ -1199,12 +1209,6 @@ OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJ
 GCPROTECT_END();
 }

 // We don't allow getting the field just so we don't have more specical
 // cases than we need to. The we need at least the throw check to insure
 // we don't allow data corruption, but
 if (!declaringType.IsNull() && Nullable::IsNullableType(pDeclMT))
 COMPlusThrow(kNotSupportedException);

 CorElementType fieldElementType = pField->GetFieldType();

 switch (fieldElementType) {
Expand Down

36 changes: 2 additions & 34 deletions
src/coreclr/src/vm/reflectioninvocation.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -328,23 +328,6 @@ FCIMPL7(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Ob

 HELPER_METHOD_FRAME_BEGIN_PROTECT(gc);

 // Verify we're not trying to set the value of a static initonly field
 // once the class has been initialized.
 if (pFieldDesc->IsStatic())
 {
 MethodTable* pEnclosingMT = pFieldDesc->GetEnclosingMethodTable();
 if (pEnclosingMT->IsClassInited() && IsFdInitOnly(pFieldDesc->GetAttributes()))
 {
 DefineFullyQualifiedNameForClassW();
 SString ssFieldName(SString::Utf8, pFieldDesc->GetName());
 COMPlusThrow(kFieldAccessException,
 IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
 ssFieldName.GetUnicode(),
 GetFullyQualifiedNameForClassW(pEnclosingMT));
 }
 }

 //TODO: cleanup this function
 InvokeUtil::SetValidField(fieldType.GetSignatureCorElementType(), fieldType, pFieldDesc, &gc.target, &gc.value, declaringType, pDomainInitialized);

 HELPER_METHOD_FRAME_END();
Expand DownExpand Up@@ -1582,7 +1565,6 @@ static void DirectObjectFieldSet(FieldDesc *pField, TypeHandle fieldType, TypeHa
 // Validate the target/fld type relationship
 InvokeUtil::ValidateObjectTarget(pField, enclosingType, &objref);

 InvokeUtil::ValidField(fieldType, pValue);
 InvokeUtil::SetValidField(pField->GetFieldType(), fieldType, pField, &objref, pValue, enclosingType, pDomainInitialized);
 GCPROTECT_END();
}
Expand DownExpand Up@@ -1626,22 +1608,8 @@ FCIMPL5(void, RuntimeFieldHandle::SetValueDirect, ReflectFieldObject *pFieldUNSA
 TypeHandle targetType = pTarget->type;
 MethodTable *pEnclosingMT = contextType.GetMethodTable();

 {
 // Verify that the value passed can be widened into the target
 InvokeUtil::ValidField(fieldType, &gc.oValue);

 // Verify we're not trying to set the value of a static initonly field
 // once the class has been initialized.
 if (pField->IsStatic() && pEnclosingMT->IsClassInited() && IsFdInitOnly(pField->GetAttributes()))
 {
 DefineFullyQualifiedNameForClassW();
 SString ssFieldName(SString::Utf8, pField->GetName());
 COMPlusThrow(kFieldAccessException,
 IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
 ssFieldName.GetUnicode(),
 GetFullyQualifiedNameForClassW(pEnclosingMT));
 }
 }
 // Verify that the value passed can be widened into the target
 InvokeUtil::ValidField(fieldType, &gc.oValue);

 CLR_BOOL domainInitialized = FALSE;
 if (pField->IsStatic() || !targetType.IsValueType()) {
Expand Down

3 changes: 3 additions & 0 deletions
src/coreclr/tests/issues.targets

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1571,6 +1571,9 @@
 <ExcludeList Include="$(XunitTestBinBase)/reflection/SetValue/TrySetReadonlyStaticField/**">
 <Issue>needs triage</Issue>
 </ExcludeList>
 <ExcludeList Include="$(XunitTestBinBase)/reflection/SetValue/TrySetReadonlyStaticField2/**">
 <Issue>https://github.com/dotnet/runtime/issues/37899</Issue>
 </ExcludeList>
 <ExcludeList Include="$(XunitTestBinBase)/Regressions/coreclr/15241/genericcontext/**">
 <Issue>needs triage</Issue>
 </ExcludeList>
Expand Down

51 changes: 51 additions & 0 deletions
src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.cs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
// 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.

using System;
using System.Reflection;

public class TestSetValue
{
 public static readonly long MagicNumber = 42;
}

public class TestSetValueDirect
{
 public static readonly string MagicString = "";
}

class Test
{
 public static int Main()
 {
 // Validate that the readonly static field cannot be set via reflection when the static constructor is triggered 
 // by the reflection SetValue operation itself.

 try
 {
 typeof(TestSetValue).GetField(nameof(TestSetValue.MagicNumber)).SetValue(null, 0x123456789);
 Console.WriteLine("FAILED: TestSetValue - Exception expected");
 return -1;
 }
 catch (FieldAccessException)
 {
 Console.WriteLine("TestSetValue - Caught expected exception");
 }

 try 
 {
 int i = 0;
 typeof(TestSetValueDirect).GetField(nameof(TestSetValueDirect.MagicString)).SetValueDirect(__makeref(i), "Hello");
 Console.WriteLine("FAILED: TestSetValueDirect - Exception expected");
 return -1;
 }
 catch (FieldAccessException)
 {
 Console.WriteLine("TestSetValueDirect - Caught expected exception");
 }
 return 100;
 }
}



9 changes: 9 additions & 0 deletions
src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.csproj

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">
 <PropertyGroup>
 <OutputType>Exe</OutputType>
 <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
 </PropertyGroup>
 <ItemGroup>
 <Compile Include="TrySetReadonlyStaticField2.cs" />
 </ItemGroup>
</Project>

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Fix detection of static readonly field re-initialization via reflection#37849

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.

 Sign up for GitHub

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

 Merged

jkotas
merged 3 commits into

from

Jun 15, 2020
 Merged


Fix detection of static readonly field re-initialization via reflection
#37849
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
5af8287

Fix detection of static readonly field re-initialization via reflection


jkotas Jun 12, 2020
2a580ac

Apply suggestions from code review


jkotas Jun 15, 2020
c055567

Fix test disable


jkotas Jun 15, 2020

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


58 changes: 31 additions & 27 deletions
src/coreclr/src/vm/invokeutil.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -941,11 +941,6 @@ void InvokeUtil::SetValidField(CorElementType fldType,
 }
 CONTRACTL_END;

 // We don't allow setting the field of nullable<T> (hasValue and value)
 // Because you can't independantly set them for this type.
 if (!declaringType.IsNull() && Nullable::IsNullableType(declaringType.GetMethodTable()))
 COMPlusThrow(kNotSupportedException);

 // call the <cinit>
 OBJECTREF Throwable = NULL;

Expand All@@ -954,27 +949,28 @@ void InvokeUtil::SetValidField(CorElementType fldType,
 {
 pDeclMT = declaringType.GetMethodTable();

 // We don't allow setting the field of nullable<T> (hasValue and value)
 // Because you can't independently set them for this type.
 if (Nullable::IsNullableType(pDeclMT))
 COMPlusThrow(kNotSupportedException);

 if (pDeclMT->IsSharedByGenericInstantiations())
 COMPlusThrow(kNotSupportedException, W("NotSupported_Type"));
 }
 else
 {
 pDeclMT = pField->GetModule()->GetGlobalMethodTable();
 }

 if (*pDomainInitialized == FALSE)
 {
 EX_TRY
 {
 if (declaringType.IsNull())
 {
 pField->GetModule()->GetGlobalMethodTable()->EnsureInstanceActive();
 pField->GetModule()->GetGlobalMethodTable()->CheckRunClassInitThrowing();
 }
 else
 {
 pDeclMT->EnsureInstanceActive();
 pDeclMT->CheckRunClassInitThrowing();

 *pDomainInitialized = TRUE;
 }
 }
 EX_CATCH_THROWABLE(&Throwable);
 }
#ifdef _DEBUG
Expand All@@ -990,6 +986,18 @@ void InvokeUtil::SetValidField(CorElementType fldType,
 GCPROTECT_END();
 }

 // Verify we're not trying to set the value of a static initonly field
 // once the class has been initialized.
 if (pField->IsStatic() && pDeclMT->IsClassInited() && IsFdInitOnly(pField->GetAttributes()))
 {
 DefineFullyQualifiedNameForClassW();
 SString ssFieldName(SString::Utf8, pField->GetName());
 COMPlusThrow(kFieldAccessException,
 IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
 ssFieldName.GetUnicode(),
 GetFullyQualifiedNameForClassW(pDeclMT));
 }

 // Set the field
 ARG_SLOT value;

Expand DownExpand Up@@ -1162,27 +1170,29 @@ OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJ
 {
 pDeclMT = declaringType.GetMethodTable();

 // We don't allow getting the field just so we don't have more specical
 // cases than we need to. Then we need at least the throw check to ensure
 // we don't allow data corruption.
 if (Nullable::IsNullableType(pDeclMT))
 COMPlusThrow(kNotSupportedException);

 if (pDeclMT->IsSharedByGenericInstantiations())
 COMPlusThrow(kNotSupportedException, W("NotSupported_Type"));
 }
 else
 {
 pDeclMT = pField->GetModule()->GetGlobalMethodTable();
 }

 if (*pDomainInitialized == FALSE)
 {
 EX_TRY
 {
 if (declaringType.IsNull())
 {
 pField->GetModule()->GetGlobalMethodTable()->EnsureInstanceActive();
 pField->GetModule()->GetGlobalMethodTable()->CheckRunClassInitThrowing();
 }
 else
 {
 pDeclMT->EnsureInstanceActive();
 pDeclMT->CheckRunClassInitThrowing();

 *pDomainInitialized = TRUE;
 }
 }
 EX_CATCH_THROWABLE(&Throwable);
 }
#ifdef _DEBUG
Expand All@@ -1199,12 +1209,6 @@ OBJECTREF InvokeUtil::GetFieldValue(FieldDesc* pField, TypeHandle fieldType, OBJ
 GCPROTECT_END();
 }

 // We don't allow getting the field just so we don't have more specical
 // cases than we need to. The we need at least the throw check to insure
 // we don't allow data corruption, but
 if (!declaringType.IsNull() && Nullable::IsNullableType(pDeclMT))
 COMPlusThrow(kNotSupportedException);

 CorElementType fieldElementType = pField->GetFieldType();

 switch (fieldElementType) {
Expand Down

36 changes: 2 additions & 34 deletions
src/coreclr/src/vm/reflectioninvocation.cpp

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -328,23 +328,6 @@ FCIMPL7(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Ob

 HELPER_METHOD_FRAME_BEGIN_PROTECT(gc);

 // Verify we're not trying to set the value of a static initonly field
 // once the class has been initialized.
 if (pFieldDesc->IsStatic())
 {
 MethodTable* pEnclosingMT = pFieldDesc->GetEnclosingMethodTable();
 if (pEnclosingMT->IsClassInited() && IsFdInitOnly(pFieldDesc->GetAttributes()))
 {
 DefineFullyQualifiedNameForClassW();
 SString ssFieldName(SString::Utf8, pFieldDesc->GetName());
 COMPlusThrow(kFieldAccessException,
 IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
 ssFieldName.GetUnicode(),
 GetFullyQualifiedNameForClassW(pEnclosingMT));
 }
 }

 //TODO: cleanup this function
 InvokeUtil::SetValidField(fieldType.GetSignatureCorElementType(), fieldType, pFieldDesc, &gc.target, &gc.value, declaringType, pDomainInitialized);

 HELPER_METHOD_FRAME_END();
Expand DownExpand Up@@ -1582,7 +1565,6 @@ static void DirectObjectFieldSet(FieldDesc *pField, TypeHandle fieldType, TypeHa
 // Validate the target/fld type relationship
 InvokeUtil::ValidateObjectTarget(pField, enclosingType, &objref);

 InvokeUtil::ValidField(fieldType, pValue);
 InvokeUtil::SetValidField(pField->GetFieldType(), fieldType, pField, &objref, pValue, enclosingType, pDomainInitialized);
 GCPROTECT_END();
}
Expand DownExpand Up@@ -1626,22 +1608,8 @@ FCIMPL5(void, RuntimeFieldHandle::SetValueDirect, ReflectFieldObject *pFieldUNSA
 TypeHandle targetType = pTarget->type;
 MethodTable *pEnclosingMT = contextType.GetMethodTable();

 {
 // Verify that the value passed can be widened into the target
 InvokeUtil::ValidField(fieldType, &gc.oValue);

 // Verify we're not trying to set the value of a static initonly field
 // once the class has been initialized.
 if (pField->IsStatic() && pEnclosingMT->IsClassInited() && IsFdInitOnly(pField->GetAttributes()))
 {
 DefineFullyQualifiedNameForClassW();
 SString ssFieldName(SString::Utf8, pField->GetName());
 COMPlusThrow(kFieldAccessException,
 IDS_EE_CANNOT_SET_INITONLY_STATIC_FIELD,
 ssFieldName.GetUnicode(),
 GetFullyQualifiedNameForClassW(pEnclosingMT));
 }
 }
 // Verify that the value passed can be widened into the target
 InvokeUtil::ValidField(fieldType, &gc.oValue);

 CLR_BOOL domainInitialized = FALSE;
 if (pField->IsStatic() || !targetType.IsValueType()) {
Expand Down

3 changes: 3 additions & 0 deletions
src/coreclr/tests/issues.targets

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1571,6 +1571,9 @@
 <ExcludeList Include="$(XunitTestBinBase)/reflection/SetValue/TrySetReadonlyStaticField/**">
 <Issue>needs triage</Issue>
 </ExcludeList>
 <ExcludeList Include="$(XunitTestBinBase)/reflection/SetValue/TrySetReadonlyStaticField2/**">
 <Issue>https://github.com/dotnet/runtime/issues/37899</Issue>
 </ExcludeList>
 <ExcludeList Include="$(XunitTestBinBase)/Regressions/coreclr/15241/genericcontext/**">
 <Issue>needs triage</Issue>
 </ExcludeList>
Expand Down

51 changes: 51 additions & 0 deletions
src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.cs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
// 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.

using System;
using System.Reflection;

public class TestSetValue
{
 public static readonly long MagicNumber = 42;
}

public class TestSetValueDirect
{
 public static readonly string MagicString = "";
}

class Test
{
 public static int Main()
 {
 // Validate that the readonly static field cannot be set via reflection when the static constructor is triggered 
 // by the reflection SetValue operation itself.

 try
 {
 typeof(TestSetValue).GetField(nameof(TestSetValue.MagicNumber)).SetValue(null, 0x123456789);
 Console.WriteLine("FAILED: TestSetValue - Exception expected");
 return -1;
 }
 catch (FieldAccessException)
 {
 Console.WriteLine("TestSetValue - Caught expected exception");
 }

 try 
 {
 int i = 0;
 typeof(TestSetValueDirect).GetField(nameof(TestSetValueDirect.MagicString)).SetValueDirect(__makeref(i), "Hello");
 Console.WriteLine("FAILED: TestSetValueDirect - Exception expected");
 return -1;
 }
 catch (FieldAccessException)
 {
 Console.WriteLine("TestSetValueDirect - Caught expected exception");
 }
 return 100;
 }
}



9 changes: 9 additions & 0 deletions
src/coreclr/tests/src/reflection/SetValue/TrySetReadonlyStaticField2.csproj

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">
 <PropertyGroup>
 <OutputType>Exe</OutputType>
 <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
 </PropertyGroup>
 <ItemGroup>
 <Compile Include="TrySetReadonlyStaticField2.cs" />
 </ItemGroup>
</Project>