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

Update And Fixes #151

Merged
merged 33 commits into from
Oct 16, 2017
Merged

Update And Fixes #151

merged 33 commits into from
Oct 16, 2017

Conversation

ThisTestUser
Copy link
Contributor

Same as #149, but updated.
-Fix the docs and readme
-Add back UnconditionalSwitchRemover for Zelix
-Improved ConstantFolder to remove impossible switches and aload pop combinations
-Fix HideAccess
-Remove VariableNormalizer because LocalVariableRemover is a better filter for illegal variables
-Add DuplicateRenamer (renames duplicate method/class/field names)
-Add more methods to JVMMethodProvider

@samczsun
Copy link
Member

I'll take a look at this in a few days - some stuff just came up and I don't have time to commit (ha ha) to any updates right now

@@ -0,0 +1,139 @@
/*
Copy link
Contributor Author

@ThisTestUser ThisTestUser Oct 14, 2017

Choose a reason for hiding this comment

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

So this normalizer fixes Zelix string decryption, but seems to have side effects.
Should we include as part of Zelix's string decrypt transformer?

Copy link
Member

Choose a reason for hiding this comment

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

If the side effects aren't important to decompilation and understanding how the code works, then leave it. If the side effects are misleading (i.e. makes parts of code appear to be run when they're not), then we should fix that before merging

@@ -249,7 +249,8 @@ public boolean canCheckEquality(JavaValue first, JavaValue second, Context conte
MethodNode decrypterNode = innerClassNode.methods.stream().filter(mn -> mn.name.equals(m.name) && mn.desc.equals(m.desc) && Modifier.isStatic(mn.access)).findFirst().orElse(null);
if (decrypterNode != null) {
Context context = new Context(provider);
context.dictionary = classpath;
context.dictionary = classpath;context.constantPools = getDeobfuscator().getConstantPools();
Copy link
Contributor

Choose a reason for hiding this comment

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

duplicate code?

@@ -231,6 +235,14 @@ public boolean transform() throws Throwable {
results.add(((Number) ((LdcFrame) frame.getComparators().get(0)).getConstant()).intValue() != ((Number) ((LdcFrame) frame.getComparators().get(1)).getConstant()).intValue());
Copy link
Contributor

@Janmm14 Janmm14 Oct 14, 2017

Choose a reason for hiding this comment

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

can't one define both int values used in all the ifs before all the if-else stuff? Would result in cleaner code.

Copy link
Member

@samczsun samczsun left a comment

Choose a reason for hiding this comment

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

Some questions

@@ -60,10 +60,6 @@ else if(arg instanceof Double)
javaArgs.add(0, new JavaDouble((Double)arg));
else if(arg instanceof Long)
javaArgs.add(0, new JavaLong((Long)arg));
else if(arg instanceof String)
Copy link
Member

Choose a reason for hiding this comment

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

Why remove these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They're not useful in any way.

@@ -0,0 +1,139 @@
/*
Copy link
Member

Choose a reason for hiding this comment

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

If the side effects aren't important to decompilation and understanding how the code works, then leave it. If the side effects are misleading (i.e. makes parts of code appear to be run when they're not), then we should fix that before merging

import com.javadeobfuscator.deobfuscator.utils.ClassTree;

@TransformerConfig.ConfigOptions(configClass = DuplicateRenamer.Config.class)
public class DuplicateRenamer extends AbstractNormalizer<DuplicateRenamer.Config>
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed? Won't the regular Class/Field/Method normalizers achieve the same result of distinguishing members?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an alternative (for me at least) to preserve the original class/method/field names as much as possible while still making it possible to extract the files.
With the introduction of the config, allowing certain files to be excluded may solve this.
For the unconditional switch, its only used to fix ZKM's string encryption at , as it sometimes can't be decompiled without that. A suggestion is to include the unconditionalswitch as part of ZKM string decryptor.


import java.util.concurrent.atomic.AtomicInteger;

public class VariableNormalizer extends Transformer<TransformerConfig> {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was removed because it was a bad solution to the illegal local variables problem. Removing them entirely worked better for decompilers.

@samczsun samczsun merged commit 20a33f7 into java-deobfuscator:master Oct 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants