Possible ArgumentOutOfRangeException when there are 65535 params in metadata table #913

New issue

Open

opened 2023-06-22 07:01:49 -05:00 by UlyssesWu · 5 comments

UlyssesWu commented

2023-06-22 07:01:49 -05:00

(Migrated from github.com)

github.com/jbevain/cecil@ba9c6c79f5/Mono.Cecil/AssemblyReader.cs (L979-L1003)

(Sorry but I'm unable to provide a sample dll.)

We have a big dll which exactly has 65535 parameters in metadata table.

When processing it using cecil, it throw exception like this:

System.ArgumentOutOfRangeException: Specified argument was out of the range of valid values.
    at Mono.Collections.Generic.Collection`1[T].get_Item (System.Int32 index)
    at Mono.Cecil.MetadataReader.ReadParameter (System.UInt32 param_rid, Mono.Cecil.MethodDefinition method)
    at Mono.Cecil.MetadataReader.ReadParameters (Mono.Cecil.MethodDefinition method, Mono.Cecil.Range param_range)
    at Mono.Cecil.MetadataReader.ReadMethod (System.UInt32 method_rid, Mono.Collections.Generic.Collection`1[T] methods)
    at Mono.Cecil.MetadataReader.ReadMethods (Mono.Cecil.TypeDefinition type)
    at Mono.Cecil.TypeDefinition+<>c.<get_Methods>b__43_0 (Mono.Cecil.TypeDefinition type, Mono.Cecil.MetadataReader reader)
    at Mono.Cecil.ModuleDefinition.Read[TItem,TRet] (TRet& variable, TItem item, System.Func`3[T1,T2,TResult] read)
    at Mono.Cecil.TypeDefinition.get_Methods ()

After some debugging I managed to locate the problem:

github.com/jbevain/cecil@ba9c6c79f5/Mono.Cecil/AssemblyReader.cs (L995)

When it's trying to read the 65535th (the last) param (it is used in a compiler-generated anonymous method), and image.GetTableIndexSize returns 2 (entry < 65536), ReadTableIndex returns 0, next_index = 0.

github.com/jbevain/cecil@ba9c6c79f5/Mono.Cecil/AssemblyReader.cs (L1000)

list.Length = next_index - start = 0u - 65535u = 4294901761 💥

On the other hand, dnlib already handled this problem, so it still works for our dll:

github.com/0xd4d/dnlib@97e07a8f1e/src/DotNet/MD/CompressedMetadata.cs (L143-L157)

The magic happens here:

https://github.com/jbevain/cecil/blob/ba9c6c79f5b82c68517cb005fb1174cea7805a64/Mono.Cecil/AssemblyReader.cs#L979-L1003 (Sorry but I'm unable to provide a sample dll.) We have a big dll which exactly has 65535 parameters in metadata table. ![image](https://github.com/jbevain/cecil/assets/4312781/51aedd41-8f47-40f8-bebe-3a344417b0fa) When processing it using cecil, it throw exception like this: ``` System.ArgumentOutOfRangeException: Specified argument was out of the range of valid values. at Mono.Collections.Generic.Collection`1[T].get_Item (System.Int32 index) at Mono.Cecil.MetadataReader.ReadParameter (System.UInt32 param_rid, Mono.Cecil.MethodDefinition method) at Mono.Cecil.MetadataReader.ReadParameters (Mono.Cecil.MethodDefinition method, Mono.Cecil.Range param_range) at Mono.Cecil.MetadataReader.ReadMethod (System.UInt32 method_rid, Mono.Collections.Generic.Collection`1[T] methods) at Mono.Cecil.MetadataReader.ReadMethods (Mono.Cecil.TypeDefinition type) at Mono.Cecil.TypeDefinition+<>c.<get_Methods>b__43_0 (Mono.Cecil.TypeDefinition type, Mono.Cecil.MetadataReader reader) at Mono.Cecil.ModuleDefinition.Read[TItem,TRet] (TRet& variable, TItem item, System.Func`3[T1,T2,TResult] read) at Mono.Cecil.TypeDefinition.get_Methods () ``` After some debugging I managed to locate the problem: https://github.com/jbevain/cecil/blob/ba9c6c79f5b82c68517cb005fb1174cea7805a64/Mono.Cecil/AssemblyReader.cs#L995 When it's trying to read the 65535th (the last) param (it is used in a compiler-generated anonymous method), and `image.GetTableIndexSize` returns 2 (entry < 65536), `ReadTableIndex` returns 0, `next_index = 0`. https://github.com/jbevain/cecil/blob/ba9c6c79f5b82c68517cb005fb1174cea7805a64/Mono.Cecil/AssemblyReader.cs#L1000 `list.Length = next_index - start = 0u - 65535u = 4294901761` 💥 ![image](https://github.com/jbevain/cecil/assets/4312781/bef14ba4-ca1a-404f-90c8-70d278d83240) On the other hand, **dnlib** already handled this problem, so it still works for our dll: https://github.com/0xd4d/dnlib/blob/97e07a8f1ea0ccbf31231dad0f7cb093805b8eee/src/DotNet/MD/CompressedMetadata.cs#L143-L157 The magic happens here: ![image](https://github.com/jbevain/cecil/assets/4312781/274bc9e4-2d28-4dc6-b682-b745c572e395)

jbevain commented

2023-06-22 19:21:05 -05:00

(Migrated from github.com)

That's a good catch, thank you :)

❤️ 1

jbevain commented

2023-06-23 11:08:30 -05:00

(Migrated from github.com)

The magic actually happens here in dnlib:

uint endRid = !hasNext || (nextListRid == 0 && tableSourceRid + 1 == tableSource.Rows && tableDest.Rows == 0xFFFF) ? lastRid : nextListRid;

The magic actually happens here in `dnlib`: ```csharp uint endRid = !hasNext || (nextListRid == 0 && tableSourceRid + 1 == tableSource.Rows && tableDest.Rows == 0xFFFF) ? lastRid : nextListRid; ```

❤️ 1

UlyssesWu commented

2023-06-23 11:38:42 -05:00

(Migrated from github.com)

uint endRid = !hasNext || (nextListRid == 0 && tableSourceRid + 1 == tableSource.Rows && tableDest.Rows == 0xFFFF) ? lastRid : nextListRid;

Thanks for pointing out. In my situation, endRid was set to 0 at this line. Then it was set to 65535 in L153, which I thought was the key to keep endRid - startRid correct in L156.

> ```cs > uint endRid = !hasNext || (nextListRid == 0 && tableSourceRid + 1 == tableSource.Rows && tableDest.Rows == 0xFFFF) ? lastRid : nextListRid; > ``` Thanks for pointing out. In my situation, `endRid` was set to 0 at this line. Then it was set to `65535` in L153, which I thought was the key to keep `endRid - startRid` correct in L156.

alexey-zakharov commented

2024-10-23 08:15:43 -05:00

(Migrated from github.com)

Are there any reasons that https://github.com/jbevain/cecil/pull/914 was not merged yet and would be feasible to use int for field indices when patching assembly with Cecil? (should I make a new issue for this?)
Thanks!

Hi! @jbevain we have a symmetric issue at Unity, but mono - Mono.Cecil generates an assembly with exactly 65535 fields and loading it in mono results in the crash due to very similar table row usage logic. Are there any reasons that https://github.com/jbevain/cecil/pull/914 was not merged yet and would be feasible to use `int` for field indices when patching assembly with Cecil? (should I make a new issue for this?) Thanks!

alexey-zakharov commented

2024-10-24 09:11:35 -05:00

(Migrated from github.com)

wrt my last comment - I checked what happens if we use large indices when table size is 65535 and it doesn't work as no reader can then read assembly properly. According to the standard large indices are only used for row count 2^16 and above