dnlib x64 exhausts all memory when read method body.

[CreateAndInject]

test.zip

static void Main(string[] args) {
        Console.WriteLine(IntPtr.Size); // Can only reproduce if current program is AnyCPU/x64
	var module = ModuleDefMD.Load("test.exe");
	var method = module.ResolveMethod(0x11d);
	var body = method.Body; // here
        Console.WriteLine(body);
	Console.ReadKey();
}

[wtfsck]

Can you paste the IL code? Could be a switch instruction.

[CreateAndInject]

dnSpy x86 can work

[wtfsck]

I don't have time to investigate at the moment. Let me know if you find something.

I'd 'nop' out instructions, save, and check if x64 version still allocs too much memory. Eventually we'd know which instruction it is, probably one of those with a token operand (eg. offset 000F).

[wwh1004]

>	dnlib.dll!dnlib.DotNet.SignatureReader.ReadSig<dnlib.DotNet.MethodSig>(dnlib.DotNet.MethodSig methodSig) Line 493	C#
 	dnlib.dll!dnlib.DotNet.SignatureReader.ReadMethod(dnlib.DotNet.CallingConvention callingConvention) Line 474	C#
 	dnlib.dll!dnlib.DotNet.SignatureReader.ReadSig() Line 434	C#
 	dnlib.dll!dnlib.DotNet.SignatureReader.ReadSig(dnlib.DotNet.ModuleDefMD readerModule, uint sig, dnlib.DotNet.GenericParamContext gpContext) Line 70	C#
 	dnlib.dll!dnlib.DotNet.ModuleDefMD.ReadSignature(uint sig, dnlib.DotNet.GenericParamContext gpContext) Line 1219	C#
 	dnlib.dll!dnlib.DotNet.StandAloneSigMD.StandAloneSigMD(dnlib.DotNet.ModuleDefMD readerModule, uint rid, dnlib.DotNet.GenericParamContext gpContext) Line 174	C#

its metadata is invalid. bp at line 'if (!reader.TryReadCompressedUInt32(out uint numParams))'
you will see 'numParams' is a extremely large number.

[CreateAndInject]

I got it :

numParams is too big

[CreateAndInject]

@wwh1004 You are one step ahead of me.
x86 throw OutOfMemoryException with 4G limit, and break on a try catch.
x64 can alloc so many memory without throw, that's why can only reproduce on x64

[wtfsck]

Since it throws, I assume that the signature is big too? What's the size of the signature blob? (see line 389 in SignatureReader.cs)

[CreateAndInject]

@wtfsck This file is really broken file.
Should we limit the value as UInt16.Max in case some program like scanner can't work?

[wtfsck]

No, it's better to detect it some other way. Limiting it to 0xFFFF or any other value will probably cause dnlib to fail to read real assemblies.

[CreateAndInject]

Seems .NET use ushort to store number of parameter/local, so why can't limit to 0xFFFF?
If we make something like file scanner, there're always invalid files on our computer, these files should not prevent the scanner from working.

[wtfsck]

OK, I misunderstood you, thinking you were referring to blob size. 0xFFFF would be too small.

It's true that you can only index up to 0xFFFF args and locals but I'm not sure what .NET (Core/Framework) will do if it finds a locals sig with more than 0xFFFF locals or a method with more than 0xFFFF args. It will probably throw an exception, in which case dnlib could also fail. Will have to check the source code later.

[CreateAndInject]

@wtfsck Seems should limit more, such as:

[wtfsck]

There are several places that allocate. AFAIK, the CLR has no generic param limit which means dnlib can't have any as well.

[CreateAndInject]

I make a test:

            var sb = new StringBuilder();
            sb.Append("class Test<");
            int max = 32768;
            for (int i = 1; i < max; i++)
                sb.Append($"T{i},");
            sb.Append($"T{max}>{{}}");
            File.WriteAllText("Test.cs", sb.ToString());

When compile the output file Test.cs, C# compiler(VBCSCompiler.exe) will crash(IndexOutOfRangeException) when greater than 32768 exactly
32768 is the count from 0 -- 32767(short.MaxValue), seems not a coincidence

[wtfsck]

That's a limitation of the C# compiler, not the runtime. IIRC, the runtime has no such limit, but if you find one let me know.

[CreateAndInject]

But the first parameter of GenericParamUser is ushort number rather than uint number

[CreateAndInject]

Since it throws, I assume that the signature is big too? What's the size of the signature blob? (see line 389 in SignatureReader.cs)

readerModule.BlobStream.StreamLeng == 90696 ,is not very large, it's strange dnlib dont occur exception when no data can read, dnlib read so many data but still don't end of the stream?

[wtfsck]

But the first parameter of GenericParamUser is ushort number rather than uint number

Yes but it doesn't mean the runtime will throw or report an error if the signature has > 0x10000 generic arguments. Have you tested this? If the runtime doesn't do anything and the code just runs, we need to support it too.

[CreateAndInject]

But the first parameter of GenericParamUser is ushort number rather than uint number

Yes but it doesn't mean the runtime will throw or report an error if the signature has > 0x10000 generic arguments. Have you tested this? If the runtime doesn't do anything and the code just runs, we need to support it too.

Yes, I test it, when run the output Test.exe .NET will throw TypeLoadException when greater than 65535: Internal limit: There're so many generic arguments. (I don't know the exact message, I translate from Chinese)
EDIT: TypeLoadException even if 0-65535 without overflow

var module = new ModuleDefUser("Test.exe");
new AssemblyDefUser("Test").Modules.Add(module);
module.Kind = ModuleKind.Console;
var type = new TypeDefUser("Program", module.CorLibTypes.Object.TypeDefOrRef);
var method = new MethodDefUser("Main", MethodSig.CreateStatic(module.CorLibTypes.Void), MethodAttributes.Public | MethodAttributes.Static);
module.EntryPoint = method;
method.Body = new CilBody();
var instrs = method.Body.Instructions;
var generic = new TypeDefUser("Generic", module.CorLibTypes.Object.TypeDefOrRef);
int max = 65536;
for (int i = 0; i < max; i++) {
	ushort number = (ushort)i;
	generic.GenericParameters.Add(new GenericParamUser(number, GenericParamAttributes.NonVariant, "T" + i));
}
module.Types.Add(generic);
instrs.Add(OpCodes.Ldtoken.ToInstruction(generic));
instrs.Add(OpCodes.Call.ToInstruction(new MemberRefUser(module, "GetTypeFromHandle", MethodSig.CreateStatic(new ClassSig(module.CorLibTypes.GetTypeRef("System", "Type")), new ValueTypeSig(module.CorLibTypes.GetTypeRef("System", "RuntimeTypeHandle"))), module.CorLibTypes.GetTypeRef("System", "Type"))));
instrs.Add(OpCodes.Call.ToInstruction(new MemberRefUser(module, "WriteLine", MethodSig.CreateStatic(module.CorLibTypes.Void, module.CorLibTypes.Object), module.CorLibTypes.GetTypeRef("System", "Console"))));
instrs.Add(OpCodes.Call.ToInstruction(new MemberRefUser(module, "ReadLine", MethodSig.CreateStatic(module.CorLibTypes.String), module.CorLibTypes.GetTypeRef("System", "Console"))));
instrs.Add(OpCodes.Ret.ToInstruction());
method.DeclaringType = type;
module.Types.Add(type);
module.Write("Test.exe");
Console.WriteLine("OK");
Console.ReadKey();

[wtfsck]

But the first parameter of GenericParamUser is ushort number rather than uint number

Yes but it doesn't mean the runtime will throw or report an error if the signature has > 0x10000 generic arguments. Have you tested this? If the runtime doesn't do anything and the code just runs, we need to support it too.

Yes, I test it, when run the output Test.exe .NET will throw TypeLoadException when greater than 65535: Internal limit: There're so many generic arguments. (I don't know the exact message, I translate from Chinese) Of course, some generic arguments have the same number, since overflow when convert int to ushort

EDIT: .NET (Core) seems to have the same error message

https://github.com/dotnet/runtime/blob/ae2e360936eb0ee737fb81c872c87dd7794782d3/src/coreclr/dlls/mscorrc/mscorrc.rc#L335

[wtfsck]

Since it throws, I assume that the signature is big too? What's the size of the signature blob? (see line 389 in SignatureReader.cs)

readerModule.BlobStream.StreamLeng == 90696 ,is not very large, it's strange dnlib dont occur exception when no data can read, dnlib read so many data but still don't end of the stream?

What happens if you change this code:

dnlib/src/DotNet/MD/BlobStream.cs

Lines 48 to 51 in 63a06e6

	public DataReader CreateReader(uint offset) {
	TryCreateReader(offset, out var reader);
	return reader;
	}

to return an empty DataReader if it fails?

[CreateAndInject]

Since it throws, I assume that the signature is big too? What's the size of the signature blob? (see line 389 in SignatureReader.cs)

readerModule.BlobStream.StreamLeng == 90696 ,is not very large, it's strange dnlib dont occur exception when no data can read, dnlib read so many data but still don't end of the stream?

What happens if you change this code:

dnlib/src/DotNet/MD/BlobStream.cs

Lines 48 to 51 in 63a06e6

	public DataReader CreateReader(uint offset) {
	TryCreateReader(offset, out var reader);
	return reader;
	}

to return an empty DataReader if it fails?

Very well, it can resolve this issue even if no limitation for big number.

[CreateAndInject]

Limitation for big number is still required when I enumerates all methods for this file

[wtfsck]

Some code that could be updated:

SignatureReader.cs:

• method param count <= 0x1_0000 (T ReadSig<T>(T methodSig) where T : MethodBaseSig {). Return null if it fails.
• locals count <= 0x1_0000 locals (LocalSig ReadLocalSig(CallingConvention callingConvention) {). Return null if it fails.
• Generic method inst GP count <= 0x1_0000 (GenericInstMethodSig ReadGenericInstMethod(CallingConvention callingConvention) {). Return null if it fails.
• Generic type inst GP count <= 0x1_0000 (case ElementType.GenericInst:). Return null if it fails.
• MDArray sizes and lowerBounds <= rank (ignore everything else) (case ElementType.Array:)

Should return an empty reader here if method call fails

dnlib/src/DotNet/MD/BlobStream.cs

Lines 48 to 51 in 63a06e6

	public DataReader CreateReader(uint offset) {
	TryCreateReader(offset, out var reader);
	return reader;
	}

CustomAttributeReader could need fixing as well since it also allocates some arrays.

I think that will solve this issue.

I don't have time to write the code at the moment, but send a PR if you want to fix one or more of the things listed above.

[CreateAndInject]

@wtfsck If something else can not be limited, you can delete the argument new List<int>(capacity), then an exception will be occured when end of reader rather than allocates so many memory that mouse and keyboard stop response.

[wtfsck]

@CreateAndInject Adding a count <= bytes left check should also work.