From 55bb20cf29d97d69aa180970bdbc85b0be3335e9 Mon Sep 17 00:00:00 2001 From: Skylot Date: Fri, 13 Aug 2021 15:43:57 +0100 Subject: [PATCH] fix: prevent collisions in method ids for java-input --- .../java/jadx/core/dex/info/InfoStorage.java | 2 +- .../java/jadx/core/dex/info/MethodInfo.java | 18 ++++++++---- .../jadx/tests/external/BaseExternalTest.java | 7 ++++- .../plugins/input/java/JavaFileLoader.java | 28 ++++++++----------- .../plugins/input/java/JavaInputPlugin.java | 2 +- .../input/java/data/ConstPoolReader.java | 2 +- .../input/java/data/JavaClassData.java | 6 ++-- .../input/java/data/JavaMethodRef.java | 12 ++++++-- .../api/plugins/input/data/IMethodRef.java | 5 ++++ 9 files changed, 51 insertions(+), 31 deletions(-) diff --git a/jadx-core/src/main/java/jadx/core/dex/info/InfoStorage.java b/jadx-core/src/main/java/jadx/core/dex/info/InfoStorage.java index c6abd7584..fdd7667d1 100644 --- a/jadx-core/src/main/java/jadx/core/dex/info/InfoStorage.java +++ b/jadx-core/src/main/java/jadx/core/dex/info/InfoStorage.java @@ -11,7 +11,7 @@ public class InfoStorage { private final Map fields = new HashMap<>(); // use only one MethodInfo instance private final Map uniqueMethods = new HashMap<>(); - // can contain same method with different ids (from different dex files) + // can contain same method with different ids (from different files) private final Map methods = new HashMap<>(); public ClassInfo getCls(ArgType type) { diff --git a/jadx-core/src/main/java/jadx/core/dex/info/MethodInfo.java b/jadx-core/src/main/java/jadx/core/dex/info/MethodInfo.java index 62881850b..f9320e81b 100644 --- a/jadx-core/src/main/java/jadx/core/dex/info/MethodInfo.java +++ b/jadx-core/src/main/java/jadx/core/dex/info/MethodInfo.java @@ -1,6 +1,10 @@ package jadx.core.dex.info; -import java.util.*; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; import org.jetbrains.annotations.Nullable; @@ -39,9 +43,11 @@ public final class MethodInfo implements Comparable { public static MethodInfo fromRef(RootNode root, IMethodRef methodRef) { InfoStorage infoStorage = root.getInfoStorage(); int uniqId = methodRef.getUniqId(); - MethodInfo prevMth = infoStorage.getByUniqId(uniqId); - if (prevMth != null) { - return prevMth; + if (uniqId != 0) { + MethodInfo prevMth = infoStorage.getByUniqId(uniqId); + if (prevMth != null) { + return prevMth; + } } methodRef.load(); ArgType parentClsType = ArgType.parse(methodRef.getParentClassType()); @@ -50,7 +56,9 @@ public final class MethodInfo implements Comparable { List args = Utils.collectionMap(methodRef.getArgTypes(), ArgType::parse); MethodInfo newMth = new MethodInfo(parentClass, methodRef.getName(), args, returnType); MethodInfo uniqMth = infoStorage.putMethod(newMth); - infoStorage.putByUniqId(uniqId, uniqMth); + if (uniqId != 0) { + infoStorage.putByUniqId(uniqId, uniqMth); + } return uniqMth; } diff --git a/jadx-core/src/test/java/jadx/tests/external/BaseExternalTest.java b/jadx-core/src/test/java/jadx/tests/external/BaseExternalTest.java index f7748f103..1d2bf2aeb 100644 --- a/jadx-core/src/test/java/jadx/tests/external/BaseExternalTest.java +++ b/jadx-core/src/test/java/jadx/tests/external/BaseExternalTest.java @@ -31,8 +31,12 @@ public abstract class BaseExternalTest extends IntegrationTest { protected abstract String getSamplesDir(); protected JadxArgs prepare(String inputFile) { + return prepare(new File(getSamplesDir(), inputFile)); + } + + protected JadxArgs prepare(File input) { JadxArgs args = new JadxArgs(); - args.getInputFiles().add(new File(getSamplesDir(), inputFile)); + args.getInputFiles().add(input); args.setOutDir(new File("../jadx-external-tests-tmp")); return args; } @@ -47,6 +51,7 @@ public abstract class BaseExternalTest extends IntegrationTest { protected void decompile(JadxArgs jadxArgs, @Nullable String clsPatternStr, @Nullable String mthPatternStr) { JadxDecompiler jadx = new JadxDecompiler(jadxArgs); + jadx.getPluginManager().unload("java-convert"); jadx.load(); if (clsPatternStr == null) { diff --git a/jadx-plugins/jadx-java-input/src/main/java/jadx/plugins/input/java/JavaFileLoader.java b/jadx-plugins/jadx-java-input/src/main/java/jadx/plugins/input/java/JavaFileLoader.java index bd3b5cb60..424d725ef 100644 --- a/jadx-plugins/jadx-java-input/src/main/java/jadx/plugins/input/java/JavaFileLoader.java +++ b/jadx-plugins/jadx-java-input/src/main/java/jadx/plugins/input/java/JavaFileLoader.java @@ -21,22 +21,22 @@ import jadx.api.plugins.utils.ZipSecurity; public class JavaFileLoader { private static final Logger LOG = LoggerFactory.getLogger(JavaFileLoader.class); - public static final int MAX_MAGIC_SIZE = 4; - public static final byte[] JAVA_CLASS_FILE_MAGIC = { (byte) 0xCA, (byte) 0xFE, (byte) 0xBA, (byte) 0xBE }; - public static final byte[] ZIP_FILE_MAGIC = { 0x50, 0x4B, 0x03, 0x04 }; + private static final int MAX_MAGIC_SIZE = 4; + private static final byte[] JAVA_CLASS_FILE_MAGIC = { (byte) 0xCA, (byte) 0xFE, (byte) 0xBA, (byte) 0xBE }; + private static final byte[] ZIP_FILE_MAGIC = { 0x50, 0x4B, 0x03, 0x04 }; - private static int classUniqId = 1; + private int classUniqId = 1; - public static List collectFiles(List inputFiles) { + public List collectFiles(List inputFiles) { return inputFiles.stream() .map(Path::toFile) - .map(JavaFileLoader::loadFromFile) + .map(this::loadFromFile) .filter(list -> !list.isEmpty()) .flatMap(Collection::stream) .collect(Collectors.toList()); } - private static List loadFromFile(File file) { + private List loadFromFile(File file) { try (InputStream inputStream = new BufferedInputStream(new FileInputStream(file))) { return loadReader(file, inputStream, file.getAbsolutePath()); } catch (Exception e) { @@ -45,7 +45,7 @@ public class JavaFileLoader { } } - private static List loadReader(File file, InputStream in, String inputFileName) throws IOException { + private List loadReader(File file, InputStream in, String inputFileName) throws IOException { byte[] magic = new byte[MAX_MAGIC_SIZE]; if (in.read(magic) != magic.length) { return Collections.emptyList(); @@ -61,7 +61,7 @@ public class JavaFileLoader { return Collections.emptyList(); } - private static List collectFromZip(File file) { + private List collectFromZip(File file) { List result = new ArrayList<>(); try { ZipSecurity.readZipEntries(file, (entry, in) -> { @@ -94,7 +94,7 @@ public class JavaFileLoader { int estimateSize = prefix.length + in.available(); ByteArrayOutputStream out = new ByteArrayOutputStream(estimateSize); out.write(prefix); - byte[] buffer = new byte[0xFFFF]; + byte[] buffer = new byte[8 * 1024]; while (true) { int len = in.read(buffer); if (len == -1) { @@ -105,11 +105,7 @@ public class JavaFileLoader { return out.toByteArray(); } - private static int getNextUniqId() { - classUniqId++; - if (classUniqId >= 0xFFFF) { - classUniqId = 1; - } - return classUniqId; + private int getNextUniqId() { + return classUniqId++; } } diff --git a/jadx-plugins/jadx-java-input/src/main/java/jadx/plugins/input/java/JavaInputPlugin.java b/jadx-plugins/jadx-java-input/src/main/java/jadx/plugins/input/java/JavaInputPlugin.java index 764a8ee41..ac7a6ce97 100644 --- a/jadx-plugins/jadx-java-input/src/main/java/jadx/plugins/input/java/JavaInputPlugin.java +++ b/jadx-plugins/jadx-java-input/src/main/java/jadx/plugins/input/java/JavaInputPlugin.java @@ -22,7 +22,7 @@ public class JavaInputPlugin implements JadxInputPlugin { @Override public ILoadResult loadFiles(List inputFiles) { - List readers = JavaFileLoader.collectFiles(inputFiles); + List readers = new JavaFileLoader().collectFiles(inputFiles); if (readers.isEmpty()) { return EmptyLoadResult.INSTANCE; } diff --git a/jadx-plugins/jadx-java-input/src/main/java/jadx/plugins/input/java/data/ConstPoolReader.java b/jadx-plugins/jadx-java-input/src/main/java/jadx/plugins/input/java/data/ConstPoolReader.java index 0b5fc181f..07d531d36 100644 --- a/jadx-plugins/jadx-java-input/src/main/java/jadx/plugins/input/java/data/ConstPoolReader.java +++ b/jadx-plugins/jadx-java-input/src/main/java/jadx/plugins/input/java/data/ConstPoolReader.java @@ -78,7 +78,7 @@ public class ConstPoolReader { int descIdx = data.readU2(); JavaMethodRef mthRef = new JavaMethodRef(); - mthRef.initUniqId(clsReader, clsIdx, nameIdx, descIdx); + mthRef.initUniqId(clsReader, idx, true); mthRef.setParentClassType(getClass(clsIdx)); mthRef.setName(getUtf8(nameIdx)); mthRef.setDescr(getUtf8(descIdx)); diff --git a/jadx-plugins/jadx-java-input/src/main/java/jadx/plugins/input/java/data/JavaClassData.java b/jadx-plugins/jadx-java-input/src/main/java/jadx/plugins/input/java/data/JavaClassData.java index a89cb5fba..a7f1b5df3 100644 --- a/jadx-plugins/jadx-java-input/src/main/java/jadx/plugins/input/java/data/JavaClassData.java +++ b/jadx-plugins/jadx-java-input/src/main/java/jadx/plugins/input/java/data/JavaClassData.java @@ -94,7 +94,7 @@ public class JavaClassData implements IClassData { methodRef.setParentClassType(classType); JavaMethodData method = new JavaMethodData(this, methodRef); for (int i = 0; i < methodsCount; i++) { - parseMethod(reader, method, clsIdx); + parseMethod(reader, method, i); mthConsumer.accept(method); } } @@ -112,7 +112,7 @@ public class JavaClassData implements IClassData { field.setAttributes(attributes); } - private void parseMethod(DataReader reader, JavaMethodData method, int clsIdx) { + private void parseMethod(DataReader reader, JavaMethodData method, int id) { int accessFlags = reader.readU2(); int nameIdx = reader.readU2(); int descriptorIdx = reader.readU2(); @@ -120,7 +120,7 @@ public class JavaClassData implements IClassData { JavaMethodRef methodRef = method.getMethodRef(); methodRef.reset(); - methodRef.initUniqId(clsReader, clsIdx, nameIdx, descriptorIdx); + methodRef.initUniqId(clsReader, id, false); methodRef.setName(constPoolReader.getUtf8(nameIdx)); methodRef.setDescr(constPoolReader.getUtf8(descriptorIdx)); diff --git a/jadx-plugins/jadx-java-input/src/main/java/jadx/plugins/input/java/data/JavaMethodRef.java b/jadx-plugins/jadx-java-input/src/main/java/jadx/plugins/input/java/data/JavaMethodRef.java index 9a614b4f5..1348ff964 100644 --- a/jadx-plugins/jadx-java-input/src/main/java/jadx/plugins/input/java/data/JavaMethodRef.java +++ b/jadx-plugins/jadx-java-input/src/main/java/jadx/plugins/input/java/data/JavaMethodRef.java @@ -16,9 +16,15 @@ public class JavaMethodRef extends JavaMethodProto implements IMethodRef { return uniqId; } - public void initUniqId(JavaClassReader clsReader, int clsIdx, int nameIdx, int descIdx) { - // TODO: check for id overlap - this.uniqId = (clsReader.getId() & 0xFFF) << 20 | (clsIdx & 0xFF) << 12 | (nameIdx & 0xFF) << 4 | descIdx & 0xF; + public void initUniqId(JavaClassReader clsReader, int id, boolean fromConstPool) { + int readerId = clsReader.getId(); + if (readerId > 0xFFFF || id > 0x7FFF) { + // loaded more than 65535 classes or more than 32767 methods in this class -> disable caching + this.uniqId = 0; + } else { + int source = fromConstPool ? 0 : 0x8000; + this.uniqId = (readerId & 0xFFFF) << 16 | source | id & 0x7FFF; + } } @Override diff --git a/jadx-plugins/jadx-plugins-api/src/main/java/jadx/api/plugins/input/data/IMethodRef.java b/jadx-plugins/jadx-plugins-api/src/main/java/jadx/api/plugins/input/data/IMethodRef.java index 6c4e6caaf..a5eadef27 100644 --- a/jadx-plugins/jadx-plugins-api/src/main/java/jadx/api/plugins/input/data/IMethodRef.java +++ b/jadx-plugins/jadx-plugins-api/src/main/java/jadx/api/plugins/input/data/IMethodRef.java @@ -4,6 +4,11 @@ import jadx.api.plugins.input.insns.custom.ICustomPayload; public interface IMethodRef extends IMethodProto, ICustomPayload { + /** + * Method unique id (will be used for caching). + * + * @return 0 if can't calculate good unique identifier (disable caching) + */ int getUniqId(); /**