fix multi-b64 comment drop in chained expressions

Author: nklapste

src/main/java/jadx/plugins/stringdecoder/B64DeobfuscatePass.java

@@ -20,6 +20,7 @@
 import jadx.core.dex.attributes.AType;
 import jadx.core.dex.instructions.ConstStringNode;
 import jadx.core.dex.instructions.FilledNewArrayNode;
+import jadx.core.dex.instructions.InvokeNode;
 import jadx.core.dex.instructions.args.InsnArg;
 import jadx.core.dex.instructions.args.RegisterArg;
 import jadx.core.dex.instructions.args.SSAVar;
@@ -135,7 +136,13 @@ public void visit(MethodNode mth) {
 				if (fieldConstants.contains(str)) {
 					continue;
 				}
-				csn.addAttr(AType.CODE_COMMENTS, new CodeComment(decoded.commentText(), CommentStyle.LINE));
+				// Attach to the top-level statement rather than the ConstStringNode so that
+				// comments from multiple decoded strings in one expression all survive.
+				// inheritMetadata uses the untyped addAttr (map.put = replace), so comments
+				// riding up through inlining chains overwrite each other; attaching directly
+				// to the statement uses the typed addAttr (list.add = merge) and bypasses that.
+				InsnNode stmtInsn = findStatementInsn(csn);
+				stmtInsn.addAttr(AType.CODE_COMMENTS, new CodeComment(decoded.commentText(), CommentStyle.LINE));
 			}
 		}
 
@@ -149,6 +156,86 @@ public void visit(MethodNode mth) {
 		}
 	}
 
+	/**
+	 * Walks the SSA def-use chain from {@code csn} upward to find the top-level statement
+	 * instruction (the one that is not itself consumed by another instruction).
+	 * Attaching CODE_COMMENTS there — using the typed, list-merging addAttr — avoids the
+	 * replace-on-copy problem in inheritMetadata when multiple decoded strings live in the
+	 * same chained expression.
+	 */
+	private static InsnNode findStatementInsn(ConstStringNode csn) {
+		RegisterArg result = csn.getResult();
+		if (result == null) {
+			return csn;
+		}
+		SSAVar var = result.getSVar();
+		if (var == null) {
+			return csn;
+		}
+		InsnNode current = csn;
+		for (int depth = 0; depth < 16; depth++) {
+			List<RegisterArg> uses = var.getUseList();
+			if (uses.size() != 1) {
+				break;
+			}
+			InsnNode useInsn = uses.get(0).getParentInsn();
+			if (useInsn == null) {
+				break;
+			}
+			RegisterArg useResult = useInsn.getResult();
+			if (useResult == null) {
+				// Constructor call (invoke-direct on <init>): the constructed object flows through
+				// arg[0] rather than a result register — pivot to its SSAVar and continue tracing.
+				if (useInsn instanceof InvokeNode && ((InvokeNode) useInsn).getCallMth().isConstructor()) {
+					InsnArg arg0 = useInsn.getArg(0);
+					if (arg0 instanceof RegisterArg) {
+						SSAVar ctorVar = ((RegisterArg) arg0).getSVar();
+						if (ctorVar != null) {
+							InsnNode postCtorUser = singlePostCtorUser(ctorVar, useInsn);
+							if (postCtorUser != null) {
+								RegisterArg postResult = postCtorUser.getResult();
+								if (postResult == null) {
+									return postCtorUser;
+								}
+								SSAVar postVar = postResult.getSVar();
+								if (postVar == null || postVar.getUseList().isEmpty()) {
+									return postCtorUser;
+								}
+								current = postCtorUser;
+								var = postVar;
+								continue;
+							}
+						}
+					}
+				}
+				return useInsn;
+			}
+			SSAVar useVar = useResult.getSVar();
+			if (useVar == null || useVar.getUseList().isEmpty()) {
+				return useInsn;
+			}
+			current = useInsn;
+			var = useVar;
+		}
+		return current;
+	}
+
+	/** Returns the single use of {@code ctorVar} that is not the constructor call itself, or null. */
+	private static InsnNode singlePostCtorUser(SSAVar ctorVar, InsnNode ctorInsn) {
+		InsnNode found = null;
+		for (RegisterArg use : ctorVar.getUseList()) {
+			InsnNode parent = use.getParentInsn();
+			if (parent == null || parent == ctorInsn) {
+				continue;
+			}
+			if (found != null) {
+				return null;
+			}
+			found = parent;
+		}
+		return found;
+	}
+
 	/** Returns the FilledNewArrayNode that directly uses the result of {@code csn}, or null. */
 	private static FilledNewArrayNode findFilledNewArrayParent(ConstStringNode csn) {
 		RegisterArg result = csn.getResult();

src/test/java/jadx/plugins/stringdecoder/JadxStringDecoderPluginTest.java

@@ -361,6 +361,18 @@ public void pascalCaseSkipDisabledTest() throws Exception {
 		assertThat(code).doesNotContain("b64:");
 	}
 
+	@Test
+	public void multiB64InvokeChainTest() throws Exception {
+		// Two const-strings both used as direct Base64.decode args in the same method chain:
+		//   Class.forName(new String(Base64.decode("...", 0)))
+		//       .getMethod(new String(Base64.decode("...", 0)), ...)
+		// Both should get b64: comments — bug: only the first was annotated
+		String code = decompileSmali("b64/multi_b64_invoke.smali");
+		System.out.println(code);
+		assertThat(code).contains("b64: android.app.ActivityThread");
+		assertThat(code).contains("b64: getPackageManager");
+	}
+
 	@Test
 	public void byteArrayStringPassDisabledTest() throws Exception {
 		// with enableByteArrayStringPass=false the bytes: comment must not appear

src/test/resources/samples/b64/multi_b64_invoke.smali

@@ -0,0 +1,43 @@
+.class public LMultiB64Invoke;
+.super Ljava/lang/Object;
+
+# Models the pattern:
+#   Class.forName(new String(Base64.decode("YW5k...", 0)))
+#       .getMethod(new String(Base64.decode("Z2V0...", 0)), new Class[0])
+#       .invoke(null, new Object[0])
+# Both const-strings are direct args to Base64.decode — both should get b64: comments.
+
+.method public static a()Ljava/lang/Object;
+    .registers 6
+
+    # "YW5kcm9pZC5hcHAuQWN0aXZpdHlUaHJlYWQ=" -> "android.app.ActivityThread"
+    const-string v0, "YW5kcm9pZC5hcHAuQWN0aXZpdHlUaHJlYWQ="
+    const/4 v1, 0x0
+    invoke-static {v0, v1}, Landroid/util/Base64;->decode(Ljava/lang/String;I)[B
+    move-result-object v0
+    new-instance v2, Ljava/lang/String;
+    invoke-direct {v2, v0}, Ljava/lang/String;-><init>([B)V
+    invoke-static {v2}, Ljava/lang/Class;->forName(Ljava/lang/String;)Ljava/lang/Class;
+    move-result-object v2
+
+    # "Z2V0UGFja2FnZU1hbmFnZXI=" -> "getPackageManager"
+    const-string v0, "Z2V0UGFja2FnZU1hbmFnZXI="
+    const/4 v1, 0x0
+    invoke-static {v0, v1}, Landroid/util/Base64;->decode(Ljava/lang/String;I)[B
+    move-result-object v0
+    new-instance v3, Ljava/lang/String;
+    invoke-direct {v3, v0}, Ljava/lang/String;-><init>([B)V
+
+    const/4 v4, 0x0
+    new-array v4, v4, [Ljava/lang/Class;
+    invoke-virtual {v2, v3, v4}, Ljava/lang/Class;->getMethod(Ljava/lang/String;[Ljava/lang/Class;)Ljava/lang/reflect/Method;
+    move-result-object v2
+
+    const/4 v3, 0x0
+    const/4 v4, 0x0
+    new-array v4, v4, [Ljava/lang/Object;
+    invoke-virtual {v2, v3, v4}, Ljava/lang/reflect/Method;->invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;
+    move-result-object v0
+
+    return-object v0
+.end method