System.Reflection.Metadata throws OOM on corrupt custom attribute value (mega array) #57531
Assignees
Projects
Milestone
Comments
dotnet-issue-labeler
bot
added
area-System.Reflection.Metadata
untriaged
|
Possibly related to #16724. We could also offer an API shape that doesn't require an allocation. |
|
Thanks for the report @joelverhagen. @steveharter do you think we can fix the call to not allocate when count is obviously wrong? Or do you think we should instead porpose a new API that can perform the check so that we can detect (without allocating) if a dll is corrupt? |
joperezr
added
bug
and removed
untriaged
joperezr
moved this from Needs triage
to Future
in Triage POD for Meta, Reflection, etc
Planned steps:
|
|
@joelverhagen do you have any other assemblies from NuGet handy that we can use for verification of a potential fix? |
|
On step 2 above (verify correctness in decoding) also see #60579 which also has issues with |
|
@steveharter, there are over 3000 packages on NuGet.org (and 8000 contained assemblies) with this problem. Here are 100.
|
Thanks! |
|
@joelverhagen can you provide a minimal repro? I assume you have an implementation of I first attempted to repro using MetadataLoadContext (since it layers on S.R.Metadata) and was able to enumerate all custom attributes on the Then I attempted repro using MetadataReader directly and was able to enumerate all attributes successfully, but did not attempt to implement |
steveharter
added
the
needs-author-action
|
This issue has been marked |
|
@steveharter, here's a minimal repro. Pass a DLL path as a command line arg. It won't OOM on all PCs so I made the app halt when memory exceeds 1 GB. reprousing System.Reflection.Metadata;
using System.Reflection.PortableExecutable;
var path = args[0];
using var fileStream = File.OpenRead(path);
using var peReader = new PEReader(fileStream);
var metadataReader = peReader.GetMetadataReader();
Console.WriteLine($"Reading {path}...");
Read(metadataReader);
static void Read(MetadataReader metadata)
{
foreach (var customAttributeHandle in metadata.GetAssemblyDefinition().GetCustomAttributes())
{
var attribute = metadata.GetCustomAttribute(customAttributeHandle);
string attributeName;
if (attribute.Constructor.Kind == HandleKind.MemberReference)
{
var memberReference = metadata.GetMemberReference((MemberReferenceHandle)attribute.Constructor);
if (memberReference.Parent.Kind == HandleKind.TypeReference)
{
var typeReference = metadata.GetTypeReference((TypeReferenceHandle)memberReference.Parent);
attributeName = metadata.GetString(typeReference.Name);
}
else if (memberReference.Parent.Kind == HandleKind.TypeDefinition)
{
var typeDefinition = metadata.GetTypeDefinition((TypeDefinitionHandle)memberReference.Parent);
attributeName = metadata.GetString(typeDefinition.Name);
}
else
{
throw new NotImplementedException("Encountered unexpected attribute constructor parent handle: " + memberReference.Parent.Kind);
}
}
else if (attribute.Constructor.Kind == HandleKind.MethodDefinition)
{
var methodDefinition = metadata.GetMethodDefinition((MethodDefinitionHandle)attribute.Constructor);
string methodName = metadata.GetString(methodDefinition.Name);
var typeDefinitionHandle = methodDefinition.GetDeclaringType();
var typeDefinition = metadata.GetTypeDefinition(typeDefinitionHandle);
attributeName = metadata.GetString(typeDefinition.Name);
if (methodName != ".ctor")
{
throw new NotImplementedException("Encountered unexpected method name: " + methodName);
}
}
else
{
throw new NotImplementedException("Encountered unexpected attribute handle: " + attribute.Constructor.Kind);
}
Console.Write($"Reading attribute '{attributeName}'...");
BlobReader blobReader = metadata.GetBlobReader(attribute.Value);
var attributeValueLength = blobReader.Length;
CustomAttributeValue<object> value;
var decoder = new TypelessDecoder();
try
{
value = attribute.DecodeValue(decoder);
Console.WriteLine(" done!");
}
catch (BadImageFormatException)
{
Console.WriteLine(" found bad image format!");
}
catch (OutOfMemoryException) when (decoder.ArrayCount > 0)
{
Console.WriteLine(" found OOM!");
}
var allocated = GC.GetTotalAllocatedBytes();
Console.WriteLine($"Memory is at {allocated / (1024.0 * 1024)} MB");
if (allocated > 1024 * 1024 * 1024)
{
Console.WriteLine("BIG MEMORY!");
return;
}
}
}
class TypelessDecoder : ICustomAttributeTypeProvider<object>
{
private int _arrayCount;
public int ArrayCount => _arrayCount;
public object GetPrimitiveType(PrimitiveTypeCode typeCode) => null;
public object GetSystemType() => null;
public object GetSZArrayType(object elementType)
{
Interlocked.Increment(ref _arrayCount);
return null;
}
public object GetTypeFromDefinition(MetadataReader reader, TypeDefinitionHandle handle, byte rawTypeKind) => null;
public object GetTypeFromReference(MetadataReader reader, TypeReferenceHandle handle, byte rawTypeKind) => null;
public object GetTypeFromSerializedName(string name) => null;
public PrimitiveTypeCode GetUnderlyingEnumType(object type) => PrimitiveTypeCode.Int32;
public bool IsSystemType(object type) => false;
}stdout |
ghost
added
needs-further-triage
|
@steveharter - I did a ninja edit of my repro and output. I realized it wasn't clearly showing the problematic attribute that causes the memory spike. |
|
@joelverhagen the implementation of |
|
Closing; please re-open if not accurate. |
steveharter
removed
the
needs-further-triage
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Description
I am using the System.Reflection.Metadata package to analyze all custom assembly attributes on NuGet.org. I am running into several strange DLLs that cause my code to OOM and I have traced it down to this line:
runtime/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/CustomAttributeDecoder.cs
Line 349 in a7e741b
For some nasty DLLs, this
countis, for example, 1,868,786,036 which causes an OOM (OutOfMemoryException) on Azure Functions. This code will inevitably throw aBadImageFormatExceptionon the subsequent for loop (there are not actually this many items in the array).Ideally, there would be a way to detect this problem prior to allocating the array or provide a hook (perhaps in the
ICustomAttributeTypeProvider<TType>) to mitigate this problem. On machines that happen to have enough memory for these mega allocations, I can catchBadImageFormatException. IMHO this exception is a good signal for such a problem.On machines where
OutOfMemoryExceptionis thrown, I am using a workaround that catchesOutOfMemoryException, if and only ifICustomAttributeTypeProvider.GetSZArrayTypehas been invoked at least once (I do this with a stateful counter in the provider implementation). Personally, I try to avoid blanket OOM catches in my code since OOMs can happen for a million different reasons.Perhaps we can avoid the pre-allocation on the
ImmutableArraybuilder if count is greater than some fuzz factor (e.g. 1024)? I don't have the data yet but it seems unlikely for have many thousands of elements in a custom attribute array argument.One example DLL with this is on NuGet.org: Kentico.Xperience.AspNet.Mvc5.Libraries 13.0.18 at path
lib/NET48/Kentico.Content.Web.Mvc.dll.Configuration
The version I am using is System.Reflection.Metadata 5.0.0.
Regression?
No
Other information
The text was updated successfully, but these errors were encountered: