From c93fe36b5ad3dd8fe9fd3aa55c358fc791745a38 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 7 Feb 2019 17:57:27 -0500 Subject: [PATCH] [sre] Handle null values in MarshalAsAttribute CustomAttributeBuilder Normally strings are encoded as length-then-utf8 in ECMA-335. But a null String is encoded as just the byte 0xFF. When we make a CustomAttributeBuilder for a MarshalAsAttribute and attach it to some parameter we first encode the CAB as a byte blob and then decode it in CAB.get_umarshal into a UnmanagedMarshal attribute. But the decoding didn't account for the possibility of null strings. Fixes https://github.com/mono/mono/issues/12747 --- .../CustomAttributeBuilder.cs | 46 ++++++++-------- .../CustomAttributeBuilderTest.cs | 54 +++++++++++++++++++ 2 files changed, 78 insertions(+), 22 deletions(-) diff --git a/mcs/class/corlib/System.Reflection.Emit/CustomAttributeBuilder.cs b/mcs/class/corlib/System.Reflection.Emit/CustomAttributeBuilder.cs index e9a8c0cbe822..66335dae624d 100644 --- a/mcs/class/corlib/System.Reflection.Emit/CustomAttributeBuilder.cs +++ b/mcs/class/corlib/System.Reflection.Emit/CustomAttributeBuilder.cs @@ -303,11 +303,24 @@ internal static string string_from_bytes (byte[] data, int pos, int len) return System.Text.Encoding.UTF8.GetString(data, pos, len); } + internal static string decode_string (byte [] data, int pos, out int rpos) + { + if (data [pos] == 0xff) { + rpos = pos + 1; + return null; + } else { + int len = decode_len (data, pos, out pos); + string s = string_from_bytes (data, pos, len); + pos += len; + rpos = pos; + return s; + } + } + internal string string_arg () { int pos = 2; - int len = decode_len (data, pos, out pos); - return string_from_bytes (data, pos, len); + return decode_string (data, pos, out pos); } internal static UnmanagedMarshal get_umarshal (CustomAttributeBuilder customBuilder, bool is_field) { @@ -335,18 +348,14 @@ internal static UnmanagedMarshal get_umarshal (CustomAttributeBuilder customBuil int paramType; // What is this ? /* Skip field/property signature */ - pos ++; + int fieldPropSig = (int)data [pos ++]; /* Read type */ paramType = ((int)data [pos++]); if (paramType == 0x55) { /* enums, the value is preceeded by the type */ - int len2 = decode_len (data, pos, out pos); - string_from_bytes (data, pos, len2); - pos += len2; + decode_string (data, pos, out pos); } - int len = decode_len (data, pos, out pos); - string named_name = string_from_bytes (data, pos, len); - pos += len; + string named_name = decode_string (data, pos, out pos); switch (named_name) { case "ArraySubType": @@ -375,9 +384,7 @@ internal static UnmanagedMarshal get_umarshal (CustomAttributeBuilder customBuil pos += 4; break; case "SafeArrayUserDefinedSubType": - len = decode_len (data, pos, out pos); - string_from_bytes (data, pos, len); - pos += len; + decode_string (data, pos, out pos); break; case "SizeParamIndex": value = (int)data [pos++]; @@ -386,20 +393,15 @@ internal static UnmanagedMarshal get_umarshal (CustomAttributeBuilder customBuil hasSize = true; break; case "MarshalType": - len = decode_len (data, pos, out pos); - marshalTypeName = string_from_bytes (data, pos, len); - pos += len; + marshalTypeName = decode_string (data, pos, out pos); break; case "MarshalTypeRef": - len = decode_len (data, pos, out pos); - marshalTypeName = string_from_bytes (data, pos, len); - marshalTypeRef = Type.GetType (marshalTypeName); - pos += len; + marshalTypeName = decode_string (data, pos, out pos); + if (marshalTypeName != null) + marshalTypeRef = Type.GetType (marshalTypeName); break; case "MarshalCookie": - len = decode_len (data, pos, out pos); - marshalCookie = string_from_bytes (data, pos, len); - pos += len; + marshalCookie = decode_string (data, pos, out pos); break; default: throw new Exception ("Unknown MarshalAsAttribute field: " + named_name); diff --git a/mcs/class/corlib/Test/System.Reflection.Emit/CustomAttributeBuilderTest.cs b/mcs/class/corlib/Test/System.Reflection.Emit/CustomAttributeBuilderTest.cs index c57616753f72..1eb0e9a3040a 100644 --- a/mcs/class/corlib/Test/System.Reflection.Emit/CustomAttributeBuilderTest.cs +++ b/mcs/class/corlib/Test/System.Reflection.Emit/CustomAttributeBuilderTest.cs @@ -9,6 +9,7 @@ using System.Reflection; using System.Reflection.Emit; using System.Threading; +using System.Runtime.InteropServices; using NUnit.Framework; namespace MonoTests.System.Reflection.Emit @@ -845,6 +846,59 @@ public void CustomAttributeAcrossAssemblies () { assemblyBuilder1.Save ("Repro55681-2a.dll"); } + [DllImport("SomeLib")] + private static extern void MethodForNullStringMarshalAsFields([MarshalAs(UnmanagedType.LPWStr)] string param); + + [Test] + public void NullStringMarshalAsFields () { + // Regression test for https://github.com/mono/mono/issues/12747 + // + // MarshalAsAttribute goes through + // CustomAttributeBuilder.get_umarshal which tries to + // build an UnmanagedMarshal value by decoding the CAB's data. + // + // The data decoding needs to handle null string (encoded as 0xFF) properly. + var aName = new AssemblyName("Repro12747"); + var assembly = AppDomain.CurrentDomain.DefineDynamicAssembly(aName, AssemblyBuilderAccess.RunAndSave, tempDir); + var module = assembly.DefineDynamicModule(aName.Name, aName.Name + ".dll"); + + var prototypeMethodName = nameof(MethodForNullStringMarshalAsFields); + var someMethod = this.GetType().GetMethod(prototypeMethodName, BindingFlags.Static | BindingFlags.NonPublic); + + var typeBuilder = module.DefineType("NewType" + module.ToString(), TypeAttributes.Class | TypeAttributes.Public); + var methodBuilder = typeBuilder.DefineMethod("NewMethod", MethodAttributes.Public | MethodAttributes.HideBySig, typeof(void), new[] { typeof(string) }); + var il = methodBuilder.GetILGenerator(); + il.Emit(OpCodes.Ret); + + var param = someMethod.GetParameters()[0]; + var paramBuilder = methodBuilder.DefineParameter(1, param.Attributes, null); + MarshalAsAttribute attr = param.GetCustomAttribute(); + var attrCtor = typeof(MarshalAsAttribute).GetConstructor(new[] { typeof(UnmanagedType) }); + object[] attrCtorArgs = { attr.Value }; + + // copy over the fields from the real MarshalAsAttribute on the parameter of "MethodForNullStringMarshalAsFields", + // including the ones that were initialized to null + var srcFields = typeof(MarshalAsAttribute).GetFields(BindingFlags.Public | BindingFlags.Instance); + var fieldArguments = new FieldInfo[srcFields.Length]; + var fieldArgumentValues = new object[srcFields.Length]; + for(int i = 0; i < srcFields.Length; i++) + { + var field = srcFields[i]; + fieldArguments[i] = field; + fieldArgumentValues[i] = field.GetValue(attr); + } + + var attrBuilder = new CustomAttributeBuilder(attrCtor, attrCtorArgs, Array.Empty(), Array.Empty(), + fieldArguments, fieldArgumentValues); + // this encodes the CustomAttributeBuilder as a data + // blob and then tries to decode it using + // CustomAttributeBuilder.get_umarshal + paramBuilder.SetCustomAttribute(attrBuilder); + + var finalType = typeBuilder.CreateType(); + + } + } }