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

Renaming fixes and improvements #305

Merged
merged 22 commits into from
Jul 2, 2021

Conversation

KvanTTT
Copy link

@KvanTTT KvanTTT commented Jan 17, 2021

Also, I've tested the new renaming algorithm on a big project and got a ~3x increase of output symbol.map size.

@AppVeyorBot
Copy link

Build ConfuserEx 677 completed (commit 17712c57ac by @KvanTTT)

@AppVeyorBot
Copy link

Build ConfuserEx 678 completed (commit 0c35448175 by @KvanTTT)

Copy link
Owner

@mkaring mkaring left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the increase in size is much of an issue. If someone wants to store the file, it can just be compressed. Even deflate should yield good results for those mapping files.

There are two changes you did, I strongly disagree with.

  1. You added the [Flags]-Attribute to the renamer modes. This is simply wrong. This enum represents fixed values and not flags. You can't combine those values in any meaningful way.
  2. The changes you did in the naming introduce a major change how generic types are handled. This will introduce problems. Please revert this part or change it to ensure that the generic name convention remains.

Confuser.Core/Utils.cs Outdated Show resolved Hide resolved
Confuser.Renamer/MessageDeobfuscator.cs Show resolved Hide resolved
Confuser.Renamer/MessageDeobfuscator.cs Outdated Show resolved Hide resolved
Confuser.Renamer/RenameMode.cs Outdated Show resolved Hide resolved
Confuser.Renamer/NameService.cs Show resolved Hide resolved
@ElektroKill
Copy link

Would it be possible to make the feature from issue #293 toggleable? This would improve customizability of the renaming protection, also some people may want to use the old renaming behavior.

@KvanTTT
Copy link
Author

KvanTTT commented Jan 18, 2021

@ElektroKill the old renaming behavior is not fully correct because of #296. Also, it makes testing more complicated. I'll take a look at how it's complex to support both behaviors.

@KvanTTT
Copy link
Author

KvanTTT commented Jan 18, 2021

I don't think the increase in size is much of an issue. If someone wants to store the file, it can just be compressed. Even deflate should yield good results for those mapping files.

Yes, I also think that's not a problem. I just reported the change before and after improvements.

@KvanTTT
Copy link
Author

KvanTTT commented Jan 18, 2021

I've added shortNames parameter that is false by default, please test.

@AppVeyorBot
Copy link

Build ConfuserEx 684 completed (commit b3644ef67e by @KvanTTT)

@mkaring
Copy link
Owner

mkaring commented Jan 18, 2021

I've added shortNames parameter that is false by default, please test.

I think you added a parameter for the wrong function. The feature to toggle is #293 (overload methods with different names) and not #296

@KvanTTT
Copy link
Author

KvanTTT commented Jan 18, 2021

They are related: short names automatically disable methods overloading and full names enable it. Take a look at the following symbols map for comparison:

Short names

_twja9ECgcldwx26TsfyApN4fSMq	Method
_w1EqeqXRRc4QS64zRqKLG0qdQyd	Class3
_phF8iy7Y79cwt3EaAFmJzW2bGch	OverloadedMethod
_P4fWEM8juOEBLCOlFBkPFc2CFeG	Method3
_zr86K1X0zsoqGU2SlY1qh6ql6XB	ToString
_wlbXOsTILjwcjRDUfpypMpJgplA	Method5
_57oALgx82oJ5doV5a3z2DkiIKcr	BaseClass5`1
_fd7vqsPkzg3XYw2JlPEdu6q0IKo	Program
_Vbxpl66DoX7BfJ6cMonbnDJZ7U	Interface5
_2q4p3nAPSd9XC4c7iHNM8jZZcdG	Class2
_jXE22YdBujEDdKdktV9VueY0sUY	Class4
_oBioa67bPo95MBJrPYiEvhJB33Z	Class5
_CV2fjIFm2SQcdBHAI10kcNoLsiN	Method2
_a7dwgGEJhWTx8qKMf4om5M6i9sG	IInterface
_GYHfKMUMLLO9oVLM117IvfCdmUC	NestedClass
_q7oQFwuoOsD2gwICwSlMR2d98mc	Main
_OatkF4GhWlgOakbgdlaLpqEglhm	Class
_2P2tMs4SkXr3PGE9kuQhD2wcM0h	BaseClass2
_mUZ1ZVjX2ukw5LiHhCvaqtpbkTg	IInterface2`1
_5eRD8xC0HjToz1A0zVlIjOx3uGP	BaseClass

Full names

_0zVaERsMluGP2G7InLbjdNFHP1L	MethodOverloading.IInterface::Method(System.String)
_M4V3KJdNqbqaKdL4zDkzValgvnS	MethodOverloading.Class3
_LzCBuBOSn49xbtKNsjuJxQZPIEW	MethodOverloading.Program::OverloadedMethod(System.Object[])
_te2aE7Jn1PSWhbO9JVKaBuSh5eK	MethodOverloading.Program::OverloadedMethod(System.Boolean,System.Single,System.Double)
_jggNgl3L13bPJVp9JdO5mVtRy1C	MethodOverloading.Class3::Method3(System.String)
_ywSbkiShk8k3qj7bBrEWEUfs9Km	MethodOverloading.Program::OverloadedMethod(System.String)
_b9ugH5wyZQqkXAl0OL44FPVcc8d	MethodOverloading.Program/NestedClass::ToString()
_qEaa2gdLuJ0h9oTr8z7bWuwF6RZ	MethodOverloading.Interface5::Method5(System.String)
_4NCMdbg8e2dwUssDmd7pyc5d9aA	MethodOverloading.BaseClass5`1
_fylTR9lTdSmz9l7DGgI9OCXvoDc	MethodOverloading.Program
_MoZbPQdTQFls0bI4c4esRZmpqZc	MethodOverloading.Interface5
_GeLgRnqCxL6farps9H57Bp7VHAe	MethodOverloading.Class2
_L2Lx026gHs2dvAzcKbUopA2h1Os	MethodOverloading.Class4
_NouXCOHEczLn28gbcMZYQfFrEOY	MethodOverloading.Class5
_tQShkhOLgB2RMcTPXJsvZDTAJOq	MethodOverloading.Program::OverloadedMethod(MethodOverloading.Program/NestedClass)
_jy6DO2hafTPBZq7Dw3AF2JjuBbcB	MethodOverloading.IInterface2`1::Method2(Result)
_iwS4d2WfWue8hAHbtgBSikKrN8w	MethodOverloading.Program::OverloadedMethod(System.Int32)
_9hUaRAWfAz286OlZiFGPTtLz4Rj	MethodOverloading.IInterface
_2bIDvpN4AADr0cuBjbDjcvQZISL	MethodOverloading.Class4::Method3(System.String)
_CZIbNVHU7wPJyGhgOcTnIUsFtC0	MethodOverloading.Program/NestedClass
_mlh9tSenWvKsazY7GGLb4a01yCm	MethodOverloading.Program::Main(System.String[])
_iyWU2GdYVZxajP8BQlt8KKTy6qQ	MethodOverloading.Class
_5V1fJdzZs5rhm6GBNT2g6vvZ3Q	MethodOverloading.BaseClass2
_v65yYwXmc3CcUSklZdTAwqP0ylI	MethodOverloading.IInterface2`1
_AuPJqi2vCONTpSZ98xIzfonc5AO	MethodOverloading.BaseClass

@AppVeyorBot
Copy link

Build ConfuserEx 685 completed (commit b35774adf4 by @KvanTTT)

@AppVeyorBot
Copy link

Build ConfuserEx 716 completed (commit 95361b3005 by @KvanTTT)

@AppVeyorBot
Copy link

Build ConfuserEx 717 completed (commit a6159f1226 by @KvanTTT)

@AppVeyorBot
Copy link

Build ConfuserEx 719 completed (commit 6afa9c6bce by @KvanTTT)

@KvanTTT
Copy link
Author

KvanTTT commented Jun 17, 2021

Any chance this request will be merged? Should I resolve conflicts?

@mkaring
Copy link
Owner

mkaring commented Jun 23, 2021

@KvanTTT: Please resolve the conflicts. I'll merge it.

@AppVeyorBot
Copy link

Build ConfuserEx 810 completed (commit 4cf3f68d7b by @KvanTTT)

@AppVeyorBot
Copy link

Build ConfuserEx 811 failed (commit cd1a2f2037 by @KvanTTT)

@AppVeyorBot
Copy link

Build ConfuserEx 812 completed (commit ae6c37f8a5 by @KvanTTT)

@KvanTTT
Copy link
Author

KvanTTT commented Jun 27, 2021

Done. Also, I fixed the renaming bug after your changes related to dynamic and fixed some potential bugs related to reflection.

@mkaring mkaring merged commit a69820a into mkaring:master Jul 2, 2021
@KvanTTT KvanTTT deleted the RenamingFixes branch July 2, 2021 10:30
@wmjordan
Copy link

I found that after this commit, ConfuserEx created a much larger but broken output for my assembly.
When the broken output assembly was run, the runtime reported some methods were not found.

I will try to make a possible smaller project to reproduce related problems.

@KvanTTT
Copy link
Author

KvanTTT commented Jul 14, 2021

I found that after this commit, ConfuserEx created a much larger but broken output for my assembly.

Do you mean symbols.map file or assembly?

I will try to make a possible smaller project to reproduce related problems.

It would be great.

@wmjordan
Copy link

I mean the assembly.
I used reversible renaming thus no symbols.map was generated.

@KvanTTT
Copy link
Author

KvanTTT commented Jul 14, 2021

Does it work with the specified shortNames option?

@wmjordan
Copy link

Not tried that option yet.

@KvanTTT
Copy link
Author

KvanTTT commented Jul 14, 2021

Please try. It's important to find out how it affects assembly.

@wmjordan
Copy link

wmjordan commented Jul 15, 2021

Well, I downloaded the artifact and used it to process my assembly without the shortNames option.
I ran the output and the runtime reported a System.BadImageFormatException that there was a duplicated type JqLoWOMO_NHYI5jO_oKtDdg.

I inspected the assembly and found up to four of them:
image

I decoded the name and found that it was not prefixed with the namespace. Yet other types were prefixed with their namespaces if their names were decoded.

Originally, those four types were of the same name, but from various namespaces.

@wmjordan
Copy link

wmjordan commented Jul 15, 2021

I managed to reproduce the problem:
If a type is referenced in an Attribute, things will go wrong.

Please use the attached configuration and assembly.

sample.zip

@wmjordan wmjordan mentioned this pull request Jul 25, 2021
@mkaring mkaring added bug Something isn't working enhancement New feature or request labels Dec 23, 2021
@mkaring mkaring added this to the 1.6 milestone Dec 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
5 participants