mkaring / ConfuserEx

An open-source, free protector for .NET applications
https://mkaring.github.io/ConfuserEx/
MIT License
2.31k stars 350 forks source link

Infinite loop detected while resolving name references #470

Closed Anthony-GitHub closed 2 years ago

Anthony-GitHub commented 2 years ago

Steps to Reproduce:

  1. Download the demo ConfuserEx2Test_InfiniteLoopDetected.zip

  2. Open the project in VS. And modify the file path for Confuser.CLI.exe in "Post-build event command line".

  3. Build

The output:

[DEBUG] Executing 'Renaming' phase...
[DEBUG] Renaming...
[ERROR] Infinite loop detected while resolving name references.
Processed definition: Original Name: ConfuserEx2Test.Internal.MyBaseClass::MyMethod(); Name Hash: 2EDCFF18
Assembly: ConfuserEx2Test.dll
Faulty References:
 - Member Override Reference(This Referenced Method(Original Name: ConfuserEx2Test.Internal.MyBaseClass::MyMethod(); Name Hash: 2EDCFF18); Base Referenced Method(Original Name: ConfuserEx2Test.Internal.MyInterfaceA::MyMethod(); Name Hash: E74373AE))
 - Member Override Reference(This Referenced Method(Original Name: ConfuserEx2Test.Internal.MyBaseClass::MyMethod(); Name Hash: 2EDCFF18); Base Referenced Method(Original Name: ConfuserEx2Test.Internal.MyInterfaceB::MyMethod(); Name Hash: 3002893D))
Anthony-GitHub commented 2 years ago

It was working with the old build "Confuser.Core 1.5.0-alpha Copyright c 2014 Ki, 2018 Martin Karing".

The changes in https://github.com/mkaring/ConfuserEx/commit/a69820a4b9a8be61966e45222fba91251ddf9efe# broken this case. The related code: https://github.com/mkaring/ConfuserEx/blob/320e90cd15ec0d06de223083b5966f6669b8dfe3/Confuser.Renamer/RenamePhase.cs#L82

If I revert this code methodDef.Name = service.ObfuscateName(methodDef, mode); to methodDef.Name = service.ObfuscateName(methodDef.Name, mode); then it can work.

mkaring commented 2 years ago

It is correct that reverting this, will make the error disappear. How ever the "issue" actually remains. Due to the change you did, ConfuserEx is just unable to detect that there is a problem. The resulting obfuscated assembly should be broken at runtime with your change.

The problem is usually caused by the way ConfuserEx processes the different assemblies. At this point the only correct way to fix this issue, is to disable the rename obfuscation for one of the affected methods (this will block the rename obfuscation for all the methods).

Anthony-GitHub commented 2 years ago

Thanks for your comment.

The problem is usually caused by the way ConfuserEx processes the different assemblies.

There is only one assembly and the code is simple:

  abstract class MyBaseClass {
    public void MyMethod() { }
  }

  public interface MyInterfaceA {
    void MyMethod();
  }

  public interface MyInterfaceB {
    void MyMethod();
  }

  class MyClassA : MyBaseClass, MyInterfaceA {
  }

  class MyClassB : MyBaseClass, MyInterfaceB {
  }

The MyMethod is defined both in interface and class and used in a class at the same time. In this case, the ConfuserEx wrongly detects an infinite loop while renaming it.

I can refactor the code as a workaround. But I still think this is a bug in ConfuserEx.

mkaring commented 2 years ago

It's a bug indeed. Caused by the sequential rename mode. I'm looking into it. The other rename modes are working fine.

Applying for example the unicode mode to the affected methods in the base class as well as both interfaces is a possible workaround.

  abstract class MyBaseClass {
    [Obfuscation(Exclude = false, Feature = "+rename(mode=Unicode)")]
    public void MyMethod() { }
  }

  public interface MyInterfaceA {
    [Obfuscation(Exclude = false, Feature = "+rename(mode=Unicode)")]
    void MyMethod();
  }

  public interface MyInterfaceB {
    [Obfuscation(Exclude = false, Feature = "+rename(mode=Unicode)")]
    void MyMethod();
  }

  class MyClassA : MyBaseClass, MyInterfaceA {
  }

  class MyClassB : MyBaseClass, MyInterfaceB {
  }
mkaring commented 2 years ago

Check out the build here. This resolves the issue for the test case.

Would be great if you could give it a spin with your real project.

Anthony-GitHub commented 2 years ago

Thank you. I test this in our real project and everything works.

github-actions[bot] commented 2 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.