From 3e9f4a50606a3cfd3abfd1e9feda9bda67285647 Mon Sep 17 00:00:00 2001 From: Skylot Date: Fri, 10 Jul 2020 18:36:17 +0100 Subject: [PATCH] fix: improve limit calculation for type updates in type inference (#854) --- .../java/jadx/core/dex/nodes/MethodNode.java | 22 +++++++++++++++---- .../typeinference/TypeInferenceVisitor.java | 15 ++++++++++--- .../visitors/typeinference/TypeUpdate.java | 7 +----- .../typeinference/TypeUpdateInfo.java | 9 ++++++++ .../java/jadx/core/utils/ErrorsCounter.java | 5 ++--- 5 files changed, 42 insertions(+), 16 deletions(-) diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/MethodNode.java b/jadx-core/src/main/java/jadx/core/dex/nodes/MethodNode.java index c52ebeb6a..26f85f3a6 100644 --- a/jadx-core/src/main/java/jadx/core/dex/nodes/MethodNode.java +++ b/jadx-core/src/main/java/jadx/core/dex/nodes/MethodNode.java @@ -49,6 +49,7 @@ public class MethodNode extends NotificationAttrNode implements IMethodDetails, private final ICodeReader codeReader; private final boolean methodIsVirtual; + private final int insnsCount; private boolean noCode; private int regsCount; @@ -85,9 +86,16 @@ public class MethodNode extends NotificationAttrNode implements IMethodDetails, this.mthInfo = MethodInfo.fromData(classNode.root(), mthData); this.parentClass = classNode; this.accFlags = new AccessInfo(mthData.getAccessFlags(), AFType.METHOD); - this.noCode = mthData.getCodeReader() == null; - this.codeReader = noCode ? null : mthData.getCodeReader().copy(); this.methodIsVirtual = !mthData.isDirect(); + ICodeReader codeReader = mthData.getCodeReader(); + this.noCode = codeReader == null; + if (noCode) { + this.codeReader = null; + this.insnsCount = 0; + } else { + this.codeReader = codeReader.copy(); + this.insnsCount = codeReader.getInsnsCount(); + } unload(); } @@ -596,8 +604,7 @@ public class MethodNode extends NotificationAttrNode implements IMethodDetails, } /** - * Stat method. - * Calculate instructions count as a measure of method size + * Calculate instructions count at currect stage */ public long countInsns() { if (instructions != null) { @@ -609,6 +616,13 @@ public class MethodNode extends NotificationAttrNode implements IMethodDetails, return -1; } + /** + * Raw instructions count in method bytecode + */ + public int getInsnsCount() { + return insnsCount; + } + @Override public boolean isVarArg() { return accFlags.isVarArgs(); diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeInferenceVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeInferenceVisitor.java index 863eebbd0..1ee143197 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeInferenceVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeInferenceVisitor.java @@ -49,6 +49,7 @@ import jadx.core.utils.BlockUtils; import jadx.core.utils.InsnList; import jadx.core.utils.InsnUtils; import jadx.core.utils.Utils; +import jadx.core.utils.exceptions.JadxOverflowException; @JadxVisitor( name = "Type Inference", @@ -88,10 +89,14 @@ public final class TypeInferenceVisitor extends AbstractVisitor { if (Consts.DEBUG_TYPE_INFERENCE) { LOG.info("Start type inference in method: {}", mth); } - for (Function resolver : resolvers) { - if (resolver.apply(mth) && checkTypes(mth)) { - return; + try { + for (Function resolver : resolvers) { + if (resolver.apply(mth) && checkTypes(mth)) { + return; + } } + } catch (Exception e) { + mth.addError("Type inference failed with exception", e); } } @@ -148,6 +153,8 @@ public final class TypeInferenceVisitor extends AbstractVisitor { if (immutableType != null) { applyImmutableType(mth, ssaVar, immutableType); } + } catch (JadxOverflowException e) { + throw e; } catch (Exception e) { LOG.error("Failed to set immutable type for var: {}", ssaVar, e); } @@ -156,6 +163,8 @@ public final class TypeInferenceVisitor extends AbstractVisitor { private boolean setBestType(MethodNode mth, SSAVar ssaVar) { try { return calculateFromBounds(mth, ssaVar); + } catch (JadxOverflowException e) { + throw e; } catch (Exception e) { LOG.error("Failed to calculate best type for var: {}", ssaVar, e); return false; diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeUpdate.java b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeUpdate.java index a690b69ba..0cf39768b 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeUpdate.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeUpdate.java @@ -166,12 +166,7 @@ public final class TypeUpdate { return CHANGED; } updateInfo.requestUpdate(arg, candidateType); - if (updateInfo.getUpdates().size() > 500) { - if (Consts.DEBUG_TYPE_INFERENCE) { - LOG.error("Type update error: too deep update tree"); - } - return REJECT; - } + updateInfo.checkUpdatesCount(); try { TypeUpdateResult result = runListeners(updateInfo, arg, candidateType); if (result == REJECT) { diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeUpdateInfo.java b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeUpdateInfo.java index b2e8911cc..7aeb017e4 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeUpdateInfo.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeUpdateInfo.java @@ -6,15 +6,18 @@ import java.util.List; import jadx.core.dex.instructions.args.ArgType; import jadx.core.dex.instructions.args.InsnArg; import jadx.core.dex.nodes.MethodNode; +import jadx.core.utils.exceptions.JadxOverflowException; public class TypeUpdateInfo { private final MethodNode mth; private final TypeUpdateFlags flags; private final List updates = new ArrayList<>(); + private final int updatesLimitCount; public TypeUpdateInfo(MethodNode mth, TypeUpdateFlags flags) { this.mth = mth; this.flags = flags; + this.updatesLimitCount = mth.getInsnsCount() * 5; // maximum registers count to update at once } public void requestUpdate(InsnArg arg, ArgType changeType) { @@ -53,6 +56,12 @@ public class TypeUpdateInfo { updates.removeIf(updateEntry -> updateEntry.getArg() == arg); } + public void checkUpdatesCount() { + if (updates.size() > updatesLimitCount) { + throw new JadxOverflowException("Type inference error: update tree size limit reached"); + } + } + public MethodNode getMth() { return mth; } diff --git a/jadx-core/src/main/java/jadx/core/utils/ErrorsCounter.java b/jadx-core/src/main/java/jadx/core/utils/ErrorsCounter.java index dd06b64cf..3689a88ba 100644 --- a/jadx-core/src/main/java/jadx/core/utils/ErrorsCounter.java +++ b/jadx-core/src/main/java/jadx/core/utils/ErrorsCounter.java @@ -51,13 +51,12 @@ public class ErrorsCounter { String msg = formatMsg(node, error); if (PRINT_MTH_SIZE && node instanceof MethodNode) { - long insnsCount = ((MethodNode) node).countInsns(); - msg = "[" + insnsCount + "] " + msg; + msg = "[" + ((MethodNode) node).getInsnsCount() + "] " + msg; } if (e == null) { LOG.error(msg); } else if (e instanceof StackOverflowError) { - LOG.error(msg); + LOG.error("{}, error: StackOverflowError", msg); } else if (e instanceof JadxOverflowException) { // don't print full stack trace String details = e.getMessage();