Mono: method-to-ir.c: Incorrect validation logic

[alxchk]

Steps to Reproduce

Minimal example:

cat > hello.il << __EOF__
.assembly Hello {}
.assembly extern mscorlib {}

.method static void Main()
{
    .entrypoint
    .maxstack 1

TEST:
    ldstr "This code should work"
    call void [mscorlib]System.Console::WriteLine(string)
    br T1

BRFALSE:
    brfalse TEST
    ret

T1:
    ldc.i4 0
    ldc.i4 0

    ceq
    br BRFALSE
}
__EOF__

ilasm /exe hello.il

Mono (invalid behavior):

Unhandled Exception:
System.InvalidProgramException: Invalid IL code in <Module>:Main (): IL_000f: brfalse   IL_0000


[ERROR] FATAL UNHANDLED EXCEPTION: System.InvalidProgramException: Invalid IL code in <Module>:Main (): IL_000f: brfalse   IL_0000

.NET (Same exe, expected behavior):

C:\TEST>hello
This code should work

On which platforms did you notice this

[x ] Linux

Version Used:

mono --version
Mono JIT compiler version 5.10.1.20 (tarball Thu Mar 29 10:39:35 UTC 2018)
Copyright (C) 2002-2014 Novell, Inc, Xamarin Inc and Contributors. www.mono-project.com
    TLS:           __thread
    SIGSEGV:       altstack
    Notifications: epoll
    Architecture:  amd64
    Disabled:      none
    Misc:          softdebug 
    Interpreter:   yes
    LLVM:          supported, not enabled.
    GC:            sgen (concurrent by default)

Reason

method-to-ir.c:mono_method_to_ir function does wrong assumptions about stack state, because translation flow does not matches interpretation flow. Thus at moment of translation actual state does not exists yet.

Unfortunately such code generated by .NETs. Such assemblies can not be executed using mono.

[jaykrell]

Can you get csc to output code like this?

[alxchk]

Very unlikely. I met such case in assemblies produced by some obfuscation program, and I don't have source code for such cases.

[filipnavara]

ECMA 335, section 1.7.5 states

III.1.7.5 Backward branch constraints

It shall be possible, with a single forward-pass through the CIL instruction stream for any
method, to infer the exact state of the evaluation stack at every instruction (where by “state” we
mean the number and type of each item on the evaluation stack).

In particular, if that single-pass analysis arrives at an instruction, call it location X, that
immediately follows an unconditional branch, and where X is not the target of an earlier branch
instruction, then the state of the evaluation stack at X, clearly, cannot be derived from existing
information. In this case, the CLI demands that the evaluation stack at X be empty.

Following on from this rule, it would clearly be invalid CIL if a later branch instruction to X
were to have a non-empty evaluation stack

[Rationale: This constraint ensures that CIL code can be processed by a simple CIL-to-native code compiler. It ensures that the state of the evaluation stack at the beginning of each CIL can
be inferred from a single, forward-pass analysis of the instruction stream. end rationale]
[Note: the stack state at location X in the above can be inferred by various means: from a
previous forward branch to X; because X marks the start of an exception handler, etc. end note]

This is clearly violated on the BRFALSE branch because it expects something on the stack. Mono is correct according to the specification. However, if you could provide more details about the obfuscation software that triggered the problem it may help us evaluate how much impact this incorrect IL has and whether it would be beneficial for Mono to allow this kind of code.

[alxchk]

Well.. What can I say. This was more than year ago. The project was to help with automatic instrumented malware processing. Idea was to stay away from the windows to do this because of obvious risks. Executables were packed malwares. Is it beneficial for Mono to allow to run obfuscated malware? I don't think so. Personally I prefer never meet this in real life. I also don't have any idea about solution used to pack those malwares, so I can't help you to measure some real-world impact.

The point was that behavior between windows .NET and mono is different. If keeping compatibility is low priority, then it's worth to close this issue.