From f62cf9a76d41e5142406013dd8aebb119526dd27 Mon Sep 17 00:00:00 2001 From: Hugh Bellamy Date: Mon, 10 Oct 2016 16:54:41 +0100 Subject: [PATCH] Fix various CustomAttributeBuilder bugs (dotnet/coreclr#7206) Fix various CustomAttributeBuilder bugs Commit migrated from https://github.com/dotnet/coreclr/commit/edc6bf9d65e84afb6425cd23ee12ca18c5029398 --- .../Reflection/Emit/CustomAttributeBuilder.cs | 98 ++++++++++++++-------- 1 file changed, 62 insertions(+), 36 deletions(-) diff --git a/src/coreclr/src/mscorlib/src/System/Reflection/Emit/CustomAttributeBuilder.cs b/src/coreclr/src/mscorlib/src/System/Reflection/Emit/CustomAttributeBuilder.cs index 2a29a5c..81ad5f3 100644 --- a/src/coreclr/src/mscorlib/src/System/Reflection/Emit/CustomAttributeBuilder.cs +++ b/src/coreclr/src/mscorlib/src/System/Reflection/Emit/CustomAttributeBuilder.cs @@ -70,8 +70,14 @@ namespace System.Reflection.Emit { // Check that a type is suitable for use in a custom attribute. private bool ValidateType(Type t) { - if (t.IsPrimitive || t == typeof(String) || t == typeof(Type)) + if (t.IsPrimitive) + { + return t != typeof(IntPtr) && t != typeof(UIntPtr); + } + if (t == typeof(String) || t == typeof(Type)) + { return true; + } if (t.IsEnum) { switch (Type.GetTypeCode(Enum.GetUnderlyingType(t))) @@ -123,7 +129,7 @@ namespace System.Reflection.Emit { if ((con.Attributes & MethodAttributes.Static) == MethodAttributes.Static || (con.Attributes & MethodAttributes.MemberAccessMask) == MethodAttributes.Private) throw new ArgumentException(Environment.GetResourceString("Argument_BadConstructor")); - + if ((con.CallingConvention & CallingConventions.Standard) != CallingConventions.Standard) throw new ArgumentException(Environment.GetResourceString("Argument_BadConstructorCallConv")); @@ -150,12 +156,16 @@ namespace System.Reflection.Emit { // Now verify that the types of the actual parameters are compatible with the types of the formal parameters. for (i = 0; i < paramTypes.Length; i++) { - if (constructorArgs[i] == null) + object constructorArg = constructorArgs[i]; + if (constructorArg == null) + { + if (paramTypes[i].IsValueType) + { + throw new ArgumentNullException($"{nameof(constructorArgs)}[{i}]"); + } continue; - TypeCode paramTC = Type.GetTypeCode(paramTypes[i]); - if (paramTC != Type.GetTypeCode(constructorArgs[i].GetType())) - if (paramTC != TypeCode.Object || !ValidateType(constructorArgs[i].GetType())) - throw new ArgumentException(Environment.GetResourceString("Argument_BadParameterTypeForConstructor", i)); + } + VerifyTypeAndPassedObjectType(paramTypes[i], constructorArg.GetType(), $"{nameof(constructorArgs)}[{i}]"); } // Allocate a memory stream to represent the CA blob in the metadata and a binary writer to help format it. @@ -176,12 +186,14 @@ namespace System.Reflection.Emit { for (i = 0; i < namedProperties.Length; i++) { // Validate the property. - if (namedProperties[i] == null) + PropertyInfo property = namedProperties[i]; + if (property == null) throw new ArgumentNullException("namedProperties[" + i + "]"); // Allow null for non-primitive types only. - Type propType = namedProperties[i].PropertyType; - if (propertyValues[i] == null && propType.IsPrimitive) + Type propType = property.PropertyType; + object propertyValue = propertyValues[i]; + if (propertyValue == null && propType.IsValueType) throw new ArgumentNullException("propertyValues[" + i + "]"); // Validate property type. @@ -189,55 +201,57 @@ namespace System.Reflection.Emit { throw new ArgumentException(Environment.GetResourceString("Argument_BadTypeInCustomAttribute")); // Property has to be writable. - if (!namedProperties[i].CanWrite) + if (!property.CanWrite) throw new ArgumentException(Environment.GetResourceString("Argument_NotAWritableProperty")); - + // Property has to be from the same class or base class as ConstructorInfo. - if (namedProperties[i].DeclaringType != con.DeclaringType + if (property.DeclaringType != con.DeclaringType && (!(con.DeclaringType is TypeBuilderInstantiation)) - && !con.DeclaringType.IsSubclassOf(namedProperties[i].DeclaringType)) + && !con.DeclaringType.IsSubclassOf(property.DeclaringType)) { // Might have failed check because one type is a XXXBuilder // and the other is not. Deal with these special cases // separately. - if (!TypeBuilder.IsTypeEqual(namedProperties[i].DeclaringType, con.DeclaringType)) + if (!TypeBuilder.IsTypeEqual(property.DeclaringType, con.DeclaringType)) { // IsSubclassOf is overloaded to do the right thing if // the constructor is a TypeBuilder, but we still need // to deal with the case where the property's declaring // type is one. - if (!(namedProperties[i].DeclaringType is TypeBuilder) || - !con.DeclaringType.IsSubclassOf(((TypeBuilder)namedProperties[i].DeclaringType).BakedRuntimeType)) + if (!(property.DeclaringType is TypeBuilder) || + !con.DeclaringType.IsSubclassOf(((TypeBuilder)property.DeclaringType).BakedRuntimeType)) throw new ArgumentException(Environment.GetResourceString("Argument_BadPropertyForConstructorBuilder")); } } // Make sure the property's type can take the given value. // Note that there will be no coersion. - if (propertyValues[i] != null && - propType != typeof(Object) && - Type.GetTypeCode(propertyValues[i].GetType()) != Type.GetTypeCode(propType)) - throw new ArgumentException(Environment.GetResourceString("Argument_ConstantDoesntMatch")); - + if (propertyValue != null) + { + VerifyTypeAndPassedObjectType(propType, propertyValue.GetType(), $"{nameof(propertyValues)}[{i}]"); + } + // First a byte indicating that this is a property. writer.Write((byte)CustomAttributeEncoding.Property); // Emit the property type, name and value. EmitType(writer, propType); EmitString(writer, namedProperties[i].Name); - EmitValue(writer, propType, propertyValues[i]); + EmitValue(writer, propType, propertyValue); } // Emit all the field sets. for (i = 0; i < namedFields.Length; i++) { // Validate the field. - if (namedFields[i] == null) + FieldInfo namedField = namedFields[i]; + if (namedField == null) throw new ArgumentNullException("namedFields[" + i + "]"); // Allow null for non-primitive types only. - Type fldType = namedFields[i].FieldType; - if (fieldValues[i] == null && fldType.IsPrimitive) + Type fldType = namedField.FieldType; + object fieldValue = fieldValues[i]; + if (fieldValue == null && fldType.IsValueType) throw new ArgumentNullException("fieldValues[" + i + "]"); // Validate field type. @@ -245,20 +259,20 @@ namespace System.Reflection.Emit { throw new ArgumentException(Environment.GetResourceString("Argument_BadTypeInCustomAttribute")); // Field has to be from the same class or base class as ConstructorInfo. - if (namedFields[i].DeclaringType != con.DeclaringType + if (namedField.DeclaringType != con.DeclaringType && (!(con.DeclaringType is TypeBuilderInstantiation)) - && !con.DeclaringType.IsSubclassOf(namedFields[i].DeclaringType)) + && !con.DeclaringType.IsSubclassOf(namedField.DeclaringType)) { // Might have failed check because one type is a XXXBuilder // and the other is not. Deal with these special cases // separately. - if (!TypeBuilder.IsTypeEqual(namedFields[i].DeclaringType, con.DeclaringType)) + if (!TypeBuilder.IsTypeEqual(namedField.DeclaringType, con.DeclaringType)) { // IsSubclassOf is overloaded to do the right thing if // the constructor is a TypeBuilder, but we still need // to deal with the case where the field's declaring // type is one. - if (!(namedFields[i].DeclaringType is TypeBuilder) || + if (!(namedField.DeclaringType is TypeBuilder) || !con.DeclaringType.IsSubclassOf(((TypeBuilder)namedFields[i].DeclaringType).BakedRuntimeType)) throw new ArgumentException(Environment.GetResourceString("Argument_BadFieldForConstructorBuilder")); } @@ -266,24 +280,36 @@ namespace System.Reflection.Emit { // Make sure the field's type can take the given value. // Note that there will be no coersion. - if (fieldValues[i] != null && - fldType != typeof(Object) && - Type.GetTypeCode(fieldValues[i].GetType()) != Type.GetTypeCode(fldType)) - throw new ArgumentException(Environment.GetResourceString("Argument_ConstantDoesntMatch")); + if (fieldValue != null) + { + VerifyTypeAndPassedObjectType(fldType, fieldValue.GetType(), $"{nameof(fieldValues)}[{i}]"); + } // First a byte indicating that this is a field. writer.Write((byte)CustomAttributeEncoding.Field); // Emit the field type, name and value. EmitType(writer, fldType); - EmitString(writer, namedFields[i].Name); - EmitValue(writer, fldType, fieldValues[i]); + EmitString(writer, namedField.Name); + EmitValue(writer, fldType, fieldValue); } // Create the blob array. m_blob = ((MemoryStream)writer.BaseStream).ToArray(); } + private static void VerifyTypeAndPassedObjectType(Type type, Type passedType, string paramName) + { + if (type != typeof(object) && Type.GetTypeCode(passedType) != Type.GetTypeCode(type)) + { + throw new ArgumentException(Environment.GetResourceString("Argument_ConstantDoesntMatch")); + } + if (passedType == typeof(IntPtr) || passedType == typeof(UIntPtr)) + { + throw new ArgumentException(Environment.GetResourceString("Argument_BadParameterTypeForCAB"), paramName); + } + } + private void EmitType(BinaryWriter writer, Type type) { if (type.IsPrimitive) -- 2.7.4