Skip to content
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.

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

System.Reflection.Emit.ParameterBuilder.SetCustomAttribute() throws undocumented ArgumentOutOfRangeException #12747

Closed
mcpiroman opened this issue Feb 2, 2019 · 3 comments

Comments

@mcpiroman
Copy link

Steps to Reproduce

Compile and run following program:

using System;
using System.Collections.Generic;
using System.Runtime.InteropServices;
using System.Reflection;
using System.Reflection.Emit;

class Program
{
    [DllImport("SomeLib")]
    private static extern void SomeMethod([MarshalAs(UnmanagedType.LPWStr)] string param);

    static void Main(string[] args)
    {
        var aName = new AssemblyName("TestAssembly");
        var assembly = AppDomain.CurrentDomain.DefineDynamicAssembly(aName, AssemblyBuilderAccess.RunAndSave);
        var module = assembly.DefineDynamicModule(aName.Name, aName.Name + ".dll");

        var someMethod = typeof(Program).GetMethod(nameof(SomeMethod), 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);
        var attr = param.GetCustomAttribute<MarshalAsAttribute>();
        var attrCtor = typeof(MarshalAsAttribute).GetConstructor(new[] { typeof(UnmanagedType) });
        object[] attrCtorArgs = { attr.Value };
        var fieldArguments = new List<FieldInfo>();
        var fieldArgumentValues = new List<object>();
        foreach (var field in typeof(MarshalAsAttribute).GetFields(BindingFlags.Public | BindingFlags.Instance))
        {
            fieldArguments.Add(field);
            fieldArgumentValues.Add(field.GetValue(attr));
        }

        var attrBuilder = new CustomAttributeBuilder(attrCtor, attrCtorArgs, Array.Empty<PropertyInfo>(), Array.Empty<object>(),
            fieldArguments.ToArray(), fieldArgumentValues.ToArray());
        paramBuilder.SetCustomAttribute(attrBuilder);

        var finalType = typeBuilder.CreateType();
        Console.WriteLine("DONE");
        Console.ReadKey(false);
    }
}

Current Behavior

System.ArgumentOutOfRangeException is thrown at line paramBuilder.SetCustomAttribute(attrBuilder);

Expected Behavior

Of course no exceptions (or at least documented ones).
Running the same program (even the same exe file) with .net framework executes everything without exception.

On which platforms did you notice this

[ ] macOS
[ ] Linux
[x] Windows

Version Used:

Mono JIT compiler version 5.18.0 (Visual Studio built mono)
Copyright (C) 2002-2014 Novell, Inc, Xamarin Inc and Contributors. www.mono-project.com
        TLS:           __thread
        SIGSEGV:       normal
        Notification:  Thread + polling
        Architecture:  amd64
        Disabled:      none
        Misc:          softdebug
        Interpreter:   yes
        LLVM:          supported, not enabled.
        Suspend:       preemptive
        GC:            sgen (concurrent by default)

Stacktrace

Unhandled Exception:
System.ArgumentOutOfRangeException: Index and count must refer to a location within the buffer.
Parameter name: bytes
  at System.Text.UTF8Encoding.GetString (System.Byte[] bytes, System.Int32 index, System.Int32 count) [0x0003e] in <08dddd7e1b69412492b591625ff22e77>:0
  at System.Reflection.Emit.CustomAttributeBuilder.string_from_bytes (System.Byte[] data, System.Int32 pos, System.Int32 len) [0x00005] in <08dddd7e1b69412492b591625ff22e77>:0
  at System.Reflection.Emit.CustomAttributeBuilder.get_umarshal (System.Reflection.Emit.CustomAttributeBuilder customBuilder, System.Boolean is_field) [0x0036c] in <08dddd7e1b69412492b591625ff22e77>:0
  at System.Reflection.Emit.ParameterBuilder.SetCustomAttribute (System.Reflection.Emit.CustomAttributeBuilder customBuilder) [0x00085] in <08dddd7e1b69412492b591625ff22e77>:0
  at Program.Main (System.String[] args) [0x00188] in <43c4d55f3f5246aea25d946fc95e4cc1>:0
[ERROR] FATAL UNHANDLED EXCEPTION: System.ArgumentOutOfRangeException: Index and count must refer to a location within the buffer.
Parameter name: bytes
  at System.Text.UTF8Encoding.GetString (System.Byte[] bytes, System.Int32 index, System.Int32 count) [0x0003e] in <08dddd7e1b69412492b591625ff22e77>:0
  at System.Reflection.Emit.CustomAttributeBuilder.string_from_bytes (System.Byte[] data, System.Int32 pos, System.Int32 len) [0x00005] in <08dddd7e1b69412492b591625ff22e77>:0
  at System.Reflection.Emit.CustomAttributeBuilder.get_umarshal (System.Reflection.Emit.CustomAttributeBuilder customBuilder, System.Boolean is_field) [0x0036c] in <08dddd7e1b69412492b591625ff22e77>:0
  at System.Reflection.Emit.ParameterBuilder.SetCustomAttribute (System.Reflection.Emit.CustomAttributeBuilder customBuilder) [0x00085] in <08dddd7e1b69412492b591625ff22e77>:0
  at Program.Main (System.String[] args) [0x00188] in <43c4d55f3f5246aea25d946fc95e4cc1>:0
@MaximLipnin
Copy link
Contributor

For the mentioned test case decode_len method returns 525536779, it leads to an exception on the next line.
data is byte[428], pos = 24, data[pos] = 255.

https://github.com/mono/mono/blob/master/mcs/class/corlib/System.Reflection.Emit/CustomAttributeBuilder.cs#L400

It's pretty old code so maybe @vargaz or @lambdageek can recall anything about it?

@lambdageek
Copy link
Member

lambdageek commented Feb 7, 2019

I'll take a look.

It kind of seems like pos is starting at an incorrect offset. Maybe an increment got lost somewhere

@lambdageek
Copy link
Member

Turns out get_umarshal doesn't know how to decode null strings (which are encoded with the single byte 0xFF in the place where the length would normally be). Working on a PR...

lambdageek added a commit to lambdageek/mono that referenced this issue Feb 7, 2019
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 mono#12747
monojenkins pushed a commit to monojenkins/mono that referenced this issue Feb 7, 2019
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 mono#12747
monojenkins pushed a commit to monojenkins/mono that referenced this issue Feb 7, 2019
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 mono#12747
monojenkins pushed a commit to monojenkins/mono that referenced this issue Feb 7, 2019
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 mono#12747
marek-safar pushed a commit that referenced this issue Feb 8, 2019
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 #12747
marek-safar pushed a commit that referenced this issue Feb 8, 2019
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 #12747
marek-safar pushed a commit that referenced this issue Feb 8, 2019
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 #12747
marek-safar pushed a commit that referenced this issue Feb 8, 2019
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 #12747
mcpiroman added a commit to mcpiroman/UnityNativeTool that referenced this issue Feb 9, 2019
The bug (mono/mono#12747) caused exception when copying MarshalAsAttribute data.
Now parameter attributes are working, though with limitations.
Removed arrays from generating hash codes (they return just pointers)
jonpryor pushed a commit to dotnet/android that referenced this issue Apr 24, 2019
Bumps to mono/api-snapshot@ae01378
Bumps to mono/reference-assemblies@e5173a5
Bumps to mono/bockbuild@d30329d
Bumps to mono/boringssl@3d87996
Bumps to mono/corefx@72f7d76
Bumps to mono/corert@1b7d4a1
Bumps to mono/helix-binaries@7e893ea
Bumps to mono/illinker-test-assets@f21ff68
Bumps to dotnet/linker@13d864e
Bumps to mono/llvm@1aaaaa5 [mono]
Bumps to mono/llvm@2c2cffe [xamarin-android]
Bumps to mono/NUnitLite@0029561
Bumps to mono/roslyn-binaries@0bbc9b4
Bumps to mono/xunit-binaries@8f6e62e

	$ git diff --shortstat 886c4901..e66c7667      # mono
        3597 files changed, 350850 insertions(+), 91128 deletions(-)
	$ git diff --shortstat 349752c464c5fc93b32e7d45825f2890c85c8b7d..2c2cffedf01e0fe266b9aaad2c2563e05b750ff4
	 240 files changed, 18562 insertions(+), 6581 deletions(-)

Context: https://github.com/dotnet/coreclr/issues/22046

Fixes: CVE 2018-8292 on macOS
Fixes: http://work.devdiv.io/737323
Fixes: https://github.com/dotnet/corefx/issues/33965
Fixes: dotnet/standard#642
Fixes: mono/mono#6997
Fixes: mono/mono#7326
Fixes: mono/mono#7517
Fixes: mono/mono#7750
Fixes: mono/mono#7859
Fixes: mono/mono#8360
Fixes: mono/mono#8460
Fixes: mono/mono#8766
Fixes: mono/mono#8922
Fixes: mono/mono#9418
Fixes: mono/mono#9507
Fixes: mono/mono#9951
Fixes: mono/mono#10024
Fixes: mono/mono#10030
Fixes: mono/mono#10038
Fixes: mono/mono#10448
Fixes: mono/mono#10735
Fixes: mono/mono#10735
Fixes: mono/mono#10737
Fixes: mono/mono#10743
Fixes: mono/mono#10834
Fixes: mono/mono#10837
Fixes: mono/mono#10838
Fixes: mono/mono#10863
Fixes: mono/mono#10945
Fixes: mono/mono#11020
Fixes: mono/mono#11021
Fixes: mono/mono#11021
Fixes: mono/mono#11049
Fixes: mono/mono#11091
Fixes: mono/mono#11095
Fixes: mono/mono#11123
Fixes: mono/mono#11138
Fixes: mono/mono#11146
Fixes: mono/mono#11202
Fixes: mono/mono#11214
Fixes: mono/mono#11317
Fixes: mono/mono#11326
Fixes: mono/mono#11378
Fixes: mono/mono#11385
Fixes: mono/mono#11478
Fixes: mono/mono#11479
Fixes: mono/mono#11488
Fixes: mono/mono#11489
Fixes: mono/mono#11527
Fixes: mono/mono#11529
Fixes: mono/mono#11596
Fixes: mono/mono#11603
Fixes: mono/mono#11613
Fixes: mono/mono#11623
Fixes: mono/mono#11663
Fixes: mono/mono#11681
Fixes: mono/mono#11684
Fixes: mono/mono#11693
Fixes: mono/mono#11697
Fixes: mono/mono#11779
Fixes: mono/mono#11809
Fixes: mono/mono#11858
Fixes: mono/mono#11895
Fixes: mono/mono#11898
Fixes: mono/mono#11898
Fixes: mono/mono#11965
Fixes: mono/mono#12182
Fixes: mono/mono#12193
Fixes: mono/mono#12218
Fixes: mono/mono#12235
Fixes: mono/mono#12263
Fixes: mono/mono#12307
Fixes: mono/mono#12331
Fixes: mono/mono#12362
Fixes: mono/mono#12374
Fixes: mono/mono#12402
Fixes: mono/mono#12421
Fixes: mono/mono#12461
Fixes: mono/mono#12479
Fixes: mono/mono#12479
Fixes: mono/mono#12552
Fixes: mono/mono#12603
Fixes: mono/mono#12747
Fixes: mono/mono#12831
Fixes: mono/mono#12843
Fixes: mono/mono#12881
Fixes: mono/mono#13030
Fixes: mono/mono#13284
Fixes: mono/mono#13297
Fixes: mono/mono#13455
Fixes: mono/mono#13460
Fixes: mono/mono#13478
Fixes: mono/mono#13479
Fixes: mono/mono#13522
Fixes: mono/mono#13607
Fixes: mono/mono#13610
Fixes: mono/mono#13610
Fixes: mono/mono#13639
Fixes: mono/mono#13672
Fixes: mono/mono#13834
Fixes: mono/mono#13878
Fixes: mono/mono#6352
Fixes: mono/monodevelop#6898
Fixes: xamarin/maccore#1069
Fixes: xamarin/maccore#1407
Fixes: xamarin/maccore#604
Fixes: xamarin/xamarin-macios#4984
Fixes: xamarin/xamarin-macios#5289
Fixes: xamarin/xamarin-macios#5363
Fixes: xamarin/xamarin-macios#5381
Fixes: https://issuetracker.unity3d.com/issues/editor-crashes-with-g-logv-when-entering-play-mode-with-active-flowcanvas-script
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants