From 8409dea980fa03c06b180969c5e0696f7cb5474b Mon Sep 17 00:00:00 2001 From: jeff Date: Sun, 21 Sep 2014 00:32:03 -0400 Subject: started unit testing for inner/anonymous class detection --- build.gradle | 47 ++++-- src/cuchaz/enigma/analysis/JarIndex.java | 181 +++++++++++++++------ src/cuchaz/enigma/mapping/SignatureUpdater.java | 18 ++ test/cuchaz/enigma/TestDeobfuscator.java | 2 +- test/cuchaz/enigma/TestInnerClasses.java | 64 ++++++++ .../enigma/TestJarIndexConstructorReferences.java | 10 +- .../cuchaz/enigma/TestJarIndexInheritanceTree.java | 14 +- test/cuchaz/enigma/TestJarIndexLoneClass.java | 14 +- .../enigma/inputs/innerClasses/Anonymous.java | 17 ++ .../inputs/innerClasses/ConstructorArgs.java | 22 +++ test/cuchaz/enigma/inputs/innerClasses/Simple.java | 9 + 11 files changed, 306 insertions(+), 92 deletions(-) create mode 100644 test/cuchaz/enigma/TestInnerClasses.java create mode 100644 test/cuchaz/enigma/inputs/innerClasses/Anonymous.java create mode 100644 test/cuchaz/enigma/inputs/innerClasses/ConstructorArgs.java create mode 100644 test/cuchaz/enigma/inputs/innerClasses/Simple.java diff --git a/build.gradle b/build.gradle index 767b032..6b89d32 100644 --- a/build.gradle +++ b/build.gradle @@ -90,21 +90,46 @@ task jarConstructors( type: Jar ) { archiveName( "testConstructors.jar" ) } -task obfTestCases( type: proguard.gradle.ProGuardTask ) { - dependsOn jarLoneClass, jarInheritanceTree, jarConstructors - +task jarInnerClasses( type: Jar ) { + from( sourceSets.test.output ) { + include( "cuchaz/enigma/inputs/Keep.class" ) + include( "cuchaz/enigma/inputs/innerClasses/**" ) + } + archiveName( "testInnerClasses.jar" ) +} + +tasks.withType( proguard.gradle.ProGuardTask ) { libraryjars( "${System.getProperty('java.home')}/lib/rt.jar" ) overloadaggressively repackageclasses allowaccessmodification dontoptimize dontshrink - keep( "class cuchaz.enigma.inputs.Keep" ) - - def jarNames = [ "LoneClass", "InheritanceTree", "Constructors" ]; - jarNames.each() { - injars( "build/libs/test${it}.jar" ) - outjars( "build/libs/test${it}.obf.jar" ) - } -} \ No newline at end of file +} + +task obfLoneClass( type: proguard.gradle.ProGuardTask, dependsOn: jarLoneClass ) { + def name = "LoneClass" + injars( "build/libs/test${name}.jar" ) + outjars( "build/libs/test${name}.obf.jar" ) +} + +task obfInheritanceTree( type: proguard.gradle.ProGuardTask, dependsOn: jarInheritanceTree ) { + def name = "InheritanceTree" + injars( "build/libs/test${name}.jar" ) + outjars( "build/libs/test${name}.obf.jar" ) +} + +task obfConstructors( type: proguard.gradle.ProGuardTask, dependsOn: jarConstructors ) { + def name = "Constructors" + injars( "build/libs/test${name}.jar" ) + outjars( "build/libs/test${name}.obf.jar" ) +} + +task obfInnerClasses( type: proguard.gradle.ProGuardTask, dependsOn: jarInnerClasses ) { + def name = "InnerClasses" + injars( "build/libs/test${name}.jar" ) + outjars( "build/libs/test${name}.obf.jar" ) +} + +task obfTestCases( dependsOn: tasks.withType( proguard.gradle.ProGuardTask ) ) diff --git a/src/cuchaz/enigma/analysis/JarIndex.java b/src/cuchaz/enigma/analysis/JarIndex.java index 6c955bc..43e2bf6 100644 --- a/src/cuchaz/enigma/analysis/JarIndex.java +++ b/src/cuchaz/enigma/analysis/JarIndex.java @@ -48,6 +48,7 @@ import cuchaz.enigma.mapping.ConstructorEntry; import cuchaz.enigma.mapping.Entry; import cuchaz.enigma.mapping.FieldEntry; import cuchaz.enigma.mapping.MethodEntry; +import cuchaz.enigma.mapping.SignatureUpdater; import cuchaz.enigma.mapping.Translator; public class JarIndex @@ -56,6 +57,7 @@ public class JarIndex private TranslationIndex m_translationIndex; private Multimap m_interfaces; private Map m_access; + private Map m_fieldClasses; private Multimap m_methodImplementations; private Multimap> m_behaviorReferences; private Multimap> m_fieldReferences; @@ -70,6 +72,7 @@ public class JarIndex m_translationIndex = new TranslationIndex(); m_interfaces = HashMultimap.create(); m_access = Maps.newHashMap(); + m_fieldClasses = Maps.newHashMap(); m_methodImplementations = HashMultimap.create(); m_behaviorReferences = HashMultimap.create(); m_fieldReferences = HashMultimap.create(); @@ -127,7 +130,7 @@ public class JarIndex } for( CtField field : c.getDeclaredFields() ) { - m_translationIndex.addField( className, field.getName() ); + indexField( field ); } for( CtBehavior behavior : c.getDeclaredBehaviors() ) { @@ -193,6 +196,22 @@ public class JarIndex EntryRenamer.renameMethodsInMultimap( m_bridgeMethods, m_fieldReferences ); } + private void indexField( CtField field ) + { + // get the field entry + String className = Descriptor.toJvmName( field.getDeclaringClass().getName() ); + FieldEntry fieldEntry = new FieldEntry( new ClassEntry( className ), field.getName() ); + + m_translationIndex.addField( className, field.getName() ); + + // is the field a class type? + if( field.getSignature().startsWith( "L" ) ) + { + ClassEntry fieldTypeEntry = new ClassEntry( field.getSignature().substring( 1, field.getSignature().length() - 1 ) ); + m_fieldClasses.put( fieldEntry, fieldTypeEntry ); + } + } + private void indexBehavior( CtBehavior behavior ) { // get the behavior entry @@ -412,7 +431,8 @@ public class JarIndex // use the synthetic fields to find the synthetic constructors for( CtConstructor constructor : c.getDeclaredConstructors() ) { - if( !isIllegalConstructor( constructor ) ) + Set syntheticFieldTypes = Sets.newHashSet(); + if( !isIllegalConstructor( syntheticFieldTypes, constructor ) ) { continue; } @@ -420,27 +440,57 @@ public class JarIndex ClassEntry classEntry = new ClassEntry( Descriptor.toJvmName( c.getName() ) ); ConstructorEntry constructorEntry = new ConstructorEntry( classEntry, constructor.getMethodInfo().getDescriptor() ); + // look at the synthetic types to get candidates for the outer class + Set candidateOuterClasses = Sets.newHashSet(); + for( String type : syntheticFieldTypes ) + { + if( type.startsWith( "L" ) ) + { + candidateOuterClasses.add( new ClassEntry( type.substring( 1, type.length() - 1 ) ) ); + } + } + + // do we have an answer yet? + if( candidateOuterClasses.isEmpty() ) + { + continue; + } + else if( candidateOuterClasses.size() == 1 ) + { + ClassEntry outerClassEntry = candidateOuterClasses.iterator().next(); + + // does this class make sense as an outer class? + if( !outerClassEntry.equals( classEntry ) ) + { + return outerClassEntry.getName(); + } + } + // who calls this constructor? Set callerClasses = Sets.newHashSet(); for( EntryReference reference : getBehaviorReferences( constructorEntry ) ) { - callerClasses.add( reference.context.getClassEntry() ); + // is that one of our candidates? + if( candidateOuterClasses.contains( reference.context.getClassEntry() ) ) + { + callerClasses.add( reference.context.getClassEntry() ); + } } - // is this called by exactly one class? + // do we have an answer yet? if( callerClasses.size() == 1 ) { - ClassEntry callerClassEntry = callerClasses.iterator().next(); + ClassEntry outerClassEntry = callerClasses.iterator().next(); // does this class make sense as an outer class? - if( !callerClassEntry.equals( classEntry ) ) + if( !outerClassEntry.equals( classEntry ) ) { - return callerClassEntry.getName(); + return outerClassEntry.getName(); } } - else if( callerClasses.size() > 1 ) + else { - System.out.println( "WARNING: Illegal constructor called by more than one class!" + callerClasses ); + System.out.println( "WARNING: Unable to choose outer class among options: " + candidateOuterClasses ); } } @@ -448,7 +498,7 @@ public class JarIndex } @SuppressWarnings( "unchecked" ) - private boolean isIllegalConstructor( CtConstructor constructor ) + private boolean isIllegalConstructor( Set syntheticFieldTypes, CtConstructor constructor ) { // illegal constructors only set synthetic member fields, then call super() String className = constructor.getDeclaringClass().getName(); @@ -456,7 +506,6 @@ public class JarIndex // collect all the field accesses, constructor calls, and method calls final List illegalFieldWrites = Lists.newArrayList(); final List constructorCalls = Lists.newArrayList(); - final List methodCalls = Lists.newArrayList(); try { constructor.instrument( new ExprEditor( ) @@ -475,12 +524,6 @@ public class JarIndex { constructorCalls.add( constructorCall ); } - - @Override - public void edit( MethodCall methodCall ) - { - methodCalls.add( methodCall ); - } } ); } catch( CannotCompileException ex ) @@ -489,25 +532,6 @@ public class JarIndex throw new Error( ex ); } - // method calls are not allowed - if( !methodCalls.isEmpty() ) - { - return false; - } - - // is there only one constructor call? - if( constructorCalls.size() != 1 ) - { - return false; - } - - // is the call to super? - ConstructorCall constructorCall = constructorCalls.get( 0 ); - if( !constructorCall.getMethodName().equals( "super" ) ) - { - return false; - } - // are there any illegal field writes? if( illegalFieldWrites.isEmpty() ) { @@ -528,7 +552,7 @@ public class JarIndex FieldInfo fieldInfo = null; for( FieldInfo info : (List)constructor.getDeclaringClass().getClassFile().getFields() ) { - if( info.getName().equals( fieldWrite.getFieldName() ) ) + if( info.getName().equals( fieldWrite.getFieldName() ) && info.getDescriptor().equals( fieldWrite.getSignature() ) ) { fieldInfo = info; break; @@ -542,9 +566,13 @@ public class JarIndex // is this field synthetic? boolean isSynthetic = (fieldInfo.getAccessFlags() & AccessFlag.SYNTHETIC) != 0; - if( !isSynthetic ) + if( isSynthetic ) + { + syntheticFieldTypes.add( fieldInfo.getDescriptor() ); + } + else { - System.err.println( String.format( "WARNING: illegal write to non synthetic field %s.%s", className, fieldInfo.getName() ) ); + System.err.println( String.format( "WARNING: illegal write to non synthetic field %s %s.%s", fieldInfo.getDescriptor(), className, fieldInfo.getName() ) ); return false; } } @@ -555,15 +583,15 @@ public class JarIndex private boolean isAnonymousClass( CtClass c, String outerClassName ) { - String innerClassName = Descriptor.toJvmName( c.getName() ); + ClassEntry innerClassEntry = new ClassEntry( Descriptor.toJvmName( c.getName() ) ); // anonymous classes: // can't be abstract // have only one constructor // it's called exactly once by the outer class - // type of inner class not referenced anywhere in outer class + // the type the instance is assigned to can't be this type - // is absract? + // is abstract? if( Modifier.isAbstract( c.getModifiers() ) ) { return false; @@ -577,22 +605,40 @@ public class JarIndex CtConstructor constructor = c.getDeclaredConstructors()[0]; // is this constructor called exactly once? - ConstructorEntry constructorEntry = new ConstructorEntry( - new ClassEntry( innerClassName ), - constructor.getMethodInfo().getDescriptor() - ); - if( getBehaviorReferences( constructorEntry ).size() != 1 ) + ConstructorEntry constructorEntry = new ConstructorEntry( innerClassEntry, constructor.getMethodInfo().getDescriptor() ); + Collection> references = getBehaviorReferences( constructorEntry ); + if( references.size() != 1 ) { return false; } - // TODO: check outer class doesn't reference type - // except this is hard because we can't just load the outer class now - // we'd have to pre-index those references in the JarIndex + // does the caller use this type? + BehaviorEntry caller = references.iterator().next().context; + for( FieldEntry fieldEntry : getReferencedFields( caller ) ) + { + ClassEntry fieldClass = getFieldClass( fieldEntry ); + if( fieldClass != null && fieldClass.equals( innerClassEntry ) ) + { + // caller references this type, so it can't be anonymous + return false; + } + } + for( BehaviorEntry behaviorEntry : getReferencedBehaviors( caller ) ) + { + // get the class types from the signature + for( String className : SignatureUpdater.getClasses( behaviorEntry.getSignature() ) ) + { + if( className.equals( innerClassEntry.getName() ) ) + { + // caller references this type, so it can't be anonymous + return false; + } + } + } return true; } - + public Set getObfClassEntries( ) { return m_obfClassEntries; @@ -608,6 +654,11 @@ public class JarIndex return m_access.get( entry ); } + public ClassEntry getFieldClass( FieldEntry fieldEntry ) + { + return m_fieldClasses.get( fieldEntry ); + } + public boolean isMethodImplemented( MethodEntry methodEntry ) { Collection implementations = m_methodImplementations.get( methodEntry.getClassName() ); @@ -772,11 +823,39 @@ public class JarIndex return m_fieldReferences.get( fieldEntry ); } + public Collection getReferencedFields( BehaviorEntry behaviorEntry ) + { + // linear search is fast enough for now + Set fieldEntries = Sets.newHashSet(); + for( EntryReference reference : m_fieldReferences.values() ) + { + if( reference.context == behaviorEntry ) + { + fieldEntries.add( reference.entry ); + } + } + return fieldEntries; + } + public Collection> getBehaviorReferences( BehaviorEntry behaviorEntry ) { return m_behaviorReferences.get( behaviorEntry ); } + public Collection getReferencedBehaviors( BehaviorEntry behaviorEntry ) + { + // linear search is fast enough for now + Set behaviorEntries = Sets.newHashSet(); + for( EntryReference reference : m_behaviorReferences.values() ) + { + if( reference.context == behaviorEntry ) + { + behaviorEntries.add( reference.entry ); + } + } + return behaviorEntries; + } + public Collection getInnerClasses( String obfOuterClassName ) { return m_innerClasses.get( obfOuterClassName ); diff --git a/src/cuchaz/enigma/mapping/SignatureUpdater.java b/src/cuchaz/enigma/mapping/SignatureUpdater.java index 4c0dbac..528a743 100644 --- a/src/cuchaz/enigma/mapping/SignatureUpdater.java +++ b/src/cuchaz/enigma/mapping/SignatureUpdater.java @@ -12,6 +12,9 @@ package cuchaz.enigma.mapping; import java.io.IOException; import java.io.StringReader; +import java.util.List; + +import com.beust.jcommander.internal.Lists; public class SignatureUpdater { @@ -84,4 +87,19 @@ public class SignatureUpdater return null; } + + public static List getClasses( String signature ) + { + final List classNames = Lists.newArrayList(); + update( signature, new ClassNameUpdater( ) + { + @Override + public String update( String className ) + { + classNames.add( className ); + return className; + } + } ); + return classNames; + } } diff --git a/test/cuchaz/enigma/TestDeobfuscator.java b/test/cuchaz/enigma/TestDeobfuscator.java index 3bb0c48..71de24a 100644 --- a/test/cuchaz/enigma/TestDeobfuscator.java +++ b/test/cuchaz/enigma/TestDeobfuscator.java @@ -11,7 +11,7 @@ ******************************************************************************/ package cuchaz.enigma; -import static org.junit.Assert.assertEquals; +import static org.junit.Assert.*; import java.io.File; import java.io.IOException; diff --git a/test/cuchaz/enigma/TestInnerClasses.java b/test/cuchaz/enigma/TestInnerClasses.java new file mode 100644 index 0000000..c6b1b5f --- /dev/null +++ b/test/cuchaz/enigma/TestInnerClasses.java @@ -0,0 +1,64 @@ +/******************************************************************************* + * Copyright (c) 2014 Jeff Martin. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the GNU Public License v3.0 + * which accompanies this distribution, and is available at + * http://www.gnu.org/licenses/gpl.html + * + * Contributors: + * Jeff Martin - initial API and implementation + ******************************************************************************/ +package cuchaz.enigma; + +import static org.hamcrest.MatcherAssert.*; +import static org.hamcrest.Matchers.*; + +import java.util.jar.JarFile; + +import org.junit.Test; + +import cuchaz.enigma.analysis.JarIndex; + +public class TestInnerClasses +{ + private JarIndex m_index; + + private static final String AnonymousOuter = "none/a"; + private static final String AnonymousInner = "none/b"; + private static final String SimpleOuter = "none/e"; + private static final String SimpleInner = "none/f"; + private static final String ConstructorArgsOuter = "none/c"; + private static final String ConstructorArgsInner = "none/d"; + + public TestInnerClasses( ) + throws Exception + { + m_index = new JarIndex(); + m_index.indexJar( new JarFile( "build/libs/testInnerClasses.obf.jar" ), true ); + } + + @Test + public void simple( ) + { + assertThat( m_index.getOuterClass( SimpleInner ), is( SimpleOuter ) ); + assertThat( m_index.getInnerClasses( SimpleOuter ), containsInAnyOrder( SimpleInner ) ); + assertThat( m_index.isAnonymousClass( SimpleInner ), is( false ) ); + } + + @Test + public void anonymous( ) + { + assertThat( m_index.getOuterClass( AnonymousInner ), is( AnonymousOuter ) ); + assertThat( m_index.getInnerClasses( AnonymousOuter ), containsInAnyOrder( AnonymousInner ) ); + assertThat( m_index.isAnonymousClass( AnonymousInner ), is( true ) ); + } + + @Test + public void constructorArgs( ) + { + assertThat( m_index.getOuterClass( ConstructorArgsInner ), is( ConstructorArgsOuter ) ); + assertThat( m_index.getInnerClasses( ConstructorArgsOuter ), containsInAnyOrder( ConstructorArgsInner ) ); + assertThat( m_index.isAnonymousClass( ConstructorArgsInner ), is( false ) ); + } +} diff --git a/test/cuchaz/enigma/TestJarIndexConstructorReferences.java b/test/cuchaz/enigma/TestJarIndexConstructorReferences.java index c069188..0238171 100644 --- a/test/cuchaz/enigma/TestJarIndexConstructorReferences.java +++ b/test/cuchaz/enigma/TestJarIndexConstructorReferences.java @@ -10,13 +10,9 @@ ******************************************************************************/ package cuchaz.enigma; -import static cuchaz.enigma.EntryFactory.newBehaviorReferenceByConstructor; -import static cuchaz.enigma.EntryFactory.newBehaviorReferenceByMethod; -import static cuchaz.enigma.EntryFactory.newClass; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.hamcrest.Matchers.empty; -import static org.hamcrest.Matchers.is; +import static cuchaz.enigma.EntryFactory.*; +import static org.hamcrest.MatcherAssert.*; +import static org.hamcrest.Matchers.*; import java.io.File; import java.util.Collection; diff --git a/test/cuchaz/enigma/TestJarIndexInheritanceTree.java b/test/cuchaz/enigma/TestJarIndexInheritanceTree.java index 3d488ee..317a6bc 100644 --- a/test/cuchaz/enigma/TestJarIndexInheritanceTree.java +++ b/test/cuchaz/enigma/TestJarIndexInheritanceTree.java @@ -10,17 +10,9 @@ ******************************************************************************/ package cuchaz.enigma; -import static cuchaz.enigma.EntryFactory.newBehaviorReferenceByConstructor; -import static cuchaz.enigma.EntryFactory.newBehaviorReferenceByMethod; -import static cuchaz.enigma.EntryFactory.newClass; -import static cuchaz.enigma.EntryFactory.newFieldReferenceByConstructor; -import static cuchaz.enigma.EntryFactory.newFieldReferenceByMethod; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.contains; -import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.hamcrest.Matchers.empty; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.nullValue; +import static cuchaz.enigma.EntryFactory.*; +import static org.hamcrest.MatcherAssert.*; +import static org.hamcrest.Matchers.*; import java.util.Collection; import java.util.Set; diff --git a/test/cuchaz/enigma/TestJarIndexLoneClass.java b/test/cuchaz/enigma/TestJarIndexLoneClass.java index 9236d0c..4c32b70 100644 --- a/test/cuchaz/enigma/TestJarIndexLoneClass.java +++ b/test/cuchaz/enigma/TestJarIndexLoneClass.java @@ -11,17 +11,9 @@ ******************************************************************************/ package cuchaz.enigma; -import static cuchaz.enigma.EntryFactory.newClass; -import static cuchaz.enigma.EntryFactory.newField; -import static cuchaz.enigma.EntryFactory.newFieldReferenceByConstructor; -import static cuchaz.enigma.EntryFactory.newFieldReferenceByMethod; -import static cuchaz.enigma.EntryFactory.newMethod; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.hamcrest.Matchers.empty; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.nullValue; +import static cuchaz.enigma.EntryFactory.*; +import static org.hamcrest.MatcherAssert.*; +import static org.hamcrest.Matchers.*; import java.util.Collection; import java.util.Set; diff --git a/test/cuchaz/enigma/inputs/innerClasses/Anonymous.java b/test/cuchaz/enigma/inputs/innerClasses/Anonymous.java new file mode 100644 index 0000000..dbff523 --- /dev/null +++ b/test/cuchaz/enigma/inputs/innerClasses/Anonymous.java @@ -0,0 +1,17 @@ +package cuchaz.enigma.inputs.innerClasses; + +public class Anonymous // a +{ + public void foo( ) + { + Runnable runnable = new Runnable( ) // b + { + @Override + public void run( ) + { + // don't care + } + }; + runnable.run(); + } +} diff --git a/test/cuchaz/enigma/inputs/innerClasses/ConstructorArgs.java b/test/cuchaz/enigma/inputs/innerClasses/ConstructorArgs.java new file mode 100644 index 0000000..d12d9cf --- /dev/null +++ b/test/cuchaz/enigma/inputs/innerClasses/ConstructorArgs.java @@ -0,0 +1,22 @@ +package cuchaz.enigma.inputs.innerClasses; + +@SuppressWarnings( "unused" ) +public class ConstructorArgs // c +{ + class Inner // d + { + private int a; + + public Inner( int a ) + { + this.a = a; + } + } + + Inner i; + + public void foo( ) + { + i = new Inner( 5 ); + } +} diff --git a/test/cuchaz/enigma/inputs/innerClasses/Simple.java b/test/cuchaz/enigma/inputs/innerClasses/Simple.java new file mode 100644 index 0000000..f2c08a7 --- /dev/null +++ b/test/cuchaz/enigma/inputs/innerClasses/Simple.java @@ -0,0 +1,9 @@ +package cuchaz.enigma.inputs.innerClasses; + +public class Simple // e +{ + class Inner // f + { + // nothing to do + } +} -- cgit v1.2.3