From 5204378fc7cccb204ae28f638935bdb4f43b3c76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Mon, 16 Aug 2021 12:32:33 +0900 Subject: [PATCH 1/4] Add handling of generic attributes to CustomAttributeDecoder If the attribute constructor refers to the generic T in its signature, we would throw `BadImageFormatException`. This adds handling by capturing the generic context and interpreting the signature variables when needed. --- .../Ecma335/CustomAttributeDecoder.cs | 144 +++++++++++++++++- 1 file changed, 139 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/CustomAttributeDecoder.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/CustomAttributeDecoder.cs index a81396cbdef16..802b6ba303d2a 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/CustomAttributeDecoder.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/CustomAttributeDecoder.cs @@ -22,6 +22,7 @@ public CustomAttributeDecoder(ICustomAttributeTypeProvider provider, Meta public CustomAttributeValue DecodeValue(EntityHandle constructor, BlobHandle value) { BlobHandle signature; + BlobHandle attributeOwningTypeSpec = default; switch (constructor.Kind) { case HandleKind.MethodDefinition: @@ -32,6 +33,13 @@ public CustomAttributeValue DecodeValue(EntityHandle constructor, BlobHan case HandleKind.MemberReference: MemberReference reference = _reader.GetMemberReference((MemberReferenceHandle)constructor); signature = reference.Signature; + + // If this is a generic attribute, we'll need its instantiation to decode the signatures + if (reference.Parent.Kind == HandleKind.TypeSpecification) + { + TypeSpecification genericOwner = _reader.GetTypeSpecification((TypeSpecificationHandle)reference.Parent); + attributeOwningTypeSpec = genericOwner.Signature; + } break; default: @@ -60,12 +68,32 @@ public CustomAttributeValue DecodeValue(EntityHandle constructor, BlobHan throw new BadImageFormatException(); } - ImmutableArray> fixedArguments = DecodeFixedArguments(ref signatureReader, ref valueReader, parameterCount); + BlobReader genericContextReader = default; + if (!attributeOwningTypeSpec.IsNil) + { + // If this is a generic attribute, grab the instantiation arguments so that we can + // interpret the constructor signature, should it refer to the generic context. + genericContextReader = _reader.GetBlobReader(attributeOwningTypeSpec); + if (genericContextReader.ReadSignatureTypeCode() == SignatureTypeCode.GenericTypeInstance) + { + int kind = genericContextReader.ReadCompressedInteger(); + if (kind != (int)SignatureTypeKind.Class && kind != (int)SignatureTypeKind.ValueType) + { + throw new BadImageFormatException(); + } + + genericContextReader.ReadTypeHandle(); + + // At this point, the reader points to the "GenArgCount Type Type*" part of the signature. + } + } + + ImmutableArray> fixedArguments = DecodeFixedArguments(ref signatureReader, ref valueReader, parameterCount, genericContextReader); ImmutableArray> namedArguments = DecodeNamedArguments(ref valueReader); return new CustomAttributeValue(fixedArguments, namedArguments); } - private ImmutableArray> DecodeFixedArguments(ref BlobReader signatureReader, ref BlobReader valueReader, int count) + private ImmutableArray> DecodeFixedArguments(ref BlobReader signatureReader, ref BlobReader valueReader, int count, BlobReader genericContextReader) { if (count == 0) { @@ -76,7 +104,7 @@ private ImmutableArray> DecodeFixedArguments for (int i = 0; i < count; i++) { - ArgumentTypeInfo info = DecodeFixedArgumentType(ref signatureReader); + ArgumentTypeInfo info = DecodeFixedArgumentType(ref signatureReader, genericContextReader); arguments.Add(DecodeArgument(ref valueReader, info)); } @@ -124,7 +152,7 @@ private struct ArgumentTypeInfo // better perf-wise, but even more important is that we can't actually reason about // a method signature with opaque TType values without adding some unnecessary chatter // with the provider. - private ArgumentTypeInfo DecodeFixedArgumentType(ref BlobReader signatureReader, bool isElementType = false) + private ArgumentTypeInfo DecodeFixedArgumentType(ref BlobReader signatureReader, BlobReader genericContextReader, bool isElementType = false) { SignatureTypeCode signatureTypeCode = signatureReader.ReadSignatureTypeCode(); @@ -170,12 +198,28 @@ private ArgumentTypeInfo DecodeFixedArgumentType(ref BlobReader signatureReader, throw new BadImageFormatException(); } - var elementInfo = DecodeFixedArgumentType(ref signatureReader, isElementType: true); + var elementInfo = DecodeFixedArgumentType(ref signatureReader, genericContextReader, isElementType: true); info.ElementType = elementInfo.Type; info.ElementTypeCode = elementInfo.TypeCode; info.Type = _provider.GetSZArrayType(info.ElementType); break; + case SignatureTypeCode.GenericTypeParameter: + int parameterIndex = signatureReader.ReadCompressedInteger(); + int numGenericParameters = genericContextReader.ReadCompressedInteger(); + if (parameterIndex >= numGenericParameters) + { + throw new BadImageFormatException(); + } + + while (parameterIndex > 0) + { + SkipType(ref genericContextReader); + parameterIndex--; + } + + return DecodeFixedArgumentType(ref genericContextReader, default, isElementType); + default: throw new BadImageFormatException(); } @@ -363,5 +407,95 @@ private TType GetTypeFromHandle(EntityHandle handle) => HandleKind.TypeReference => _provider.GetTypeFromReference(_reader, (TypeReferenceHandle)handle, 0), _ => throw new BadImageFormatException(SR.NotTypeDefOrRefHandle), }; + + private static void SkipType(ref BlobReader blobReader) + { + int typeCode = blobReader.ReadCompressedInteger(); + + switch (typeCode) + { + case (int)SignatureTypeCode.Boolean: + case (int)SignatureTypeCode.Char: + case (int)SignatureTypeCode.SByte: + case (int)SignatureTypeCode.Byte: + case (int)SignatureTypeCode.Int16: + case (int)SignatureTypeCode.UInt16: + case (int)SignatureTypeCode.Int32: + case (int)SignatureTypeCode.UInt32: + case (int)SignatureTypeCode.Int64: + case (int)SignatureTypeCode.UInt64: + case (int)SignatureTypeCode.Single: + case (int)SignatureTypeCode.Double: + case (int)SignatureTypeCode.IntPtr: + case (int)SignatureTypeCode.UIntPtr: + case (int)SignatureTypeCode.Object: + case (int)SignatureTypeCode.String: + case (int)SignatureTypeCode.Void: + case (int)SignatureTypeCode.TypedReference: + return; + + case (int)SignatureTypeCode.Pointer: + case (int)SignatureTypeCode.ByReference: + case (int)SignatureTypeCode.Pinned: + case (int)SignatureTypeCode.SZArray: + SkipType(ref blobReader); + return; + + case (int)SignatureTypeCode.FunctionPointer: + SignatureHeader header = blobReader.ReadSignatureHeader(); + if (header.IsGeneric) + { + blobReader.ReadCompressedInteger(); // arity + } + + int paramCount = blobReader.ReadCompressedInteger(); + SkipType(ref blobReader); + for (int i = 0; i < paramCount; i++) + SkipType(ref blobReader); + return; + + case (int)SignatureTypeCode.Array: + SkipType(ref blobReader); + blobReader.ReadCompressedInteger(); // rank + int boundsCount = blobReader.ReadCompressedInteger(); + for (int i = 0; i < boundsCount; i++) + { + blobReader.ReadCompressedInteger(); + } + int lowerBoundsCount = blobReader.ReadCompressedInteger(); + for (int i = 0; i < lowerBoundsCount; i++) + { + blobReader.ReadCompressedSignedInteger(); + } + return; + + case (int)SignatureTypeCode.RequiredModifier: + case (int)SignatureTypeCode.OptionalModifier: + blobReader.ReadTypeHandle(); + SkipType(ref blobReader); + return; + + case (int)SignatureTypeCode.GenericTypeInstance: + SkipType(ref blobReader); + int count = blobReader.ReadCompressedInteger(); + for (int i = 0; i < count; i++) + { + SkipType(ref blobReader); + } + return; + + case (int)SignatureTypeCode.GenericTypeParameter: + blobReader.ReadCompressedInteger(); + return; + + case (int)SignatureTypeKind.Class: + case (int)SignatureTypeKind.ValueType: + SkipType(ref blobReader); + break; + + default: + throw new BadImageFormatException(); + } + } } } From 878d9ae7e46bcbb3db8fa42dc7bcd5d7aea54d3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 17 Aug 2021 07:01:23 +0900 Subject: [PATCH 2/4] Update CustomAttributeDecoder.cs --- .../Metadata/Ecma335/CustomAttributeDecoder.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/CustomAttributeDecoder.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/CustomAttributeDecoder.cs index 802b6ba303d2a..b4777bcbbeb2c 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/CustomAttributeDecoder.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/CustomAttributeDecoder.cs @@ -86,6 +86,12 @@ public CustomAttributeValue DecodeValue(EntityHandle constructor, BlobHan // At this point, the reader points to the "GenArgCount Type Type*" part of the signature. } + else + { + // Some other invalid TypeSpec. Don't accidentally allow resolving generic parameters + // from the constructor signature into a broken blob. + genericContextReader = default; + } } ImmutableArray> fixedArguments = DecodeFixedArguments(ref signatureReader, ref valueReader, parameterCount, genericContextReader); @@ -205,6 +211,11 @@ private ArgumentTypeInfo DecodeFixedArgumentType(ref BlobReader signatureReader, break; case SignatureTypeCode.GenericTypeParameter: + if (genericContextReader.IsNil) + { + throw new BadImageFormatException(); + } + int parameterIndex = signatureReader.ReadCompressedInteger(); int numGenericParameters = genericContextReader.ReadCompressedInteger(); if (parameterIndex >= numGenericParameters) From 52b42706f973d80b195fdf229d5e8c5521854024 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 17 Aug 2021 09:03:52 +0900 Subject: [PATCH 3/4] Update CustomAttributeDecoder.cs --- .../Reflection/Metadata/Ecma335/CustomAttributeDecoder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/CustomAttributeDecoder.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/CustomAttributeDecoder.cs index b4777bcbbeb2c..de7a3150d702f 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/CustomAttributeDecoder.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/CustomAttributeDecoder.cs @@ -211,7 +211,7 @@ private ArgumentTypeInfo DecodeFixedArgumentType(ref BlobReader signatureReader, break; case SignatureTypeCode.GenericTypeParameter: - if (genericContextReader.IsNil) + if (genericContextReader.Length == 0) { throw new BadImageFormatException(); } From e60bc797b975a45aea271c2510afb95538d1cb08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 17 Aug 2021 11:07:59 +0900 Subject: [PATCH 4/4] Update CustomAttributeDecoder.cs --- .../Reflection/Metadata/Ecma335/CustomAttributeDecoder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/CustomAttributeDecoder.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/CustomAttributeDecoder.cs index de7a3150d702f..45deeee785304 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/CustomAttributeDecoder.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/CustomAttributeDecoder.cs @@ -215,7 +215,7 @@ private ArgumentTypeInfo DecodeFixedArgumentType(ref BlobReader signatureReader, { throw new BadImageFormatException(); } - + int parameterIndex = signatureReader.ReadCompressedInteger(); int numGenericParameters = genericContextReader.ReadCompressedInteger(); if (parameterIndex >= numGenericParameters)