Skip to content

Commit

Permalink
fix: various minor improvements (PR #1418)
Browse files Browse the repository at this point in the history 
* chore: better variable naming for getInstance calls
* chore: rebalance preferences window and fix empty plugins section directly after jadx-gui start
* chore: do not ask for project save if nothing had been changed
* use parallel mode for gradle
* minor improvements for app debugging
* apply CodeQL suggestion to prevent log injection
* handle IntelliJ Idea warnings
* replace not-ASCII chars in LogUtils.escape

Co-authored-by: Skylot <skylot@gmail.com>
  • Loading branch information
@jpstotz @skylot
jpstotz and skylot authored Mar 23, 2022
1 parent 8fe1ee1 commit 909cf0a
Show file tree
Hide file tree
Showing 15 changed files with 193 additions and 88 deletions.
1 change: 1 addition & 0 deletions gradle.properties
View file
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
org.gradle.warning.mode=all
org.gradle.parallel=true
6 changes: 5 additions & 1 deletion jadx-core/src/main/java/jadx/core/codegen/NameGen.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,13 +268,17 @@ private String makeNameFromInsn(InsnNode insn) {

private String makeNameFromInvoke(MethodInfo callMth) {
String name = callMth.getName();
ArgType declType = callMth.getDeclClass().getType();
if ("getInstance".equals(name)) {
// e.g. Cipher.getInstance
return makeNameForType(declType);
}
if (name.startsWith("get") || name.startsWith("set")) {
return fromName(name.substring(3));
}
if ("iterator".equals(name)) {
return "it";
}
ArgType declType = callMth.getDeclClass().getType();
if ("toString".equals(name)) {
return makeNameForType(declType);
}
Expand Down
30 changes: 30 additions & 0 deletions jadx-core/src/main/java/jadx/core/utils/log/LogUtils.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package jadx.core.utils.log;

import java.nio.charset.StandardCharsets;
import java.util.regex.Pattern;

/**
* Escape input from untrusted source before pass to logger.
* Suggested by CodeQL: https://codeql.github.com/codeql-query-help/java/java-log-injection/
*/
public class LogUtils {

private static final Pattern ALFA_NUMERIC = Pattern.compile("\\w*");

public static String escape(String input) {
if (input == null) {
return "null";
}
if (ALFA_NUMERIC.matcher(input).matches()) {
return input;
}
return input.replaceAll("\\W", ".");
}

public static String escape(byte[] input) {
if (input == null) {
return "null";
}
return escape(new String(input, StandardCharsets.UTF_8));
}
}
13 changes: 13 additions & 0 deletions jadx-core/src/test/java/jadx/core/utils/log/LogUtilsTest.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package jadx.core.utils.log;

import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;

class LogUtilsTest {

@Test
void escape() {
assertThat(LogUtils.escape("Guest'%0AUser:'Admin")).isEqualTo("Guest..0AUser..Admin");
}
}
13 changes: 7 additions & 6 deletions jadx-gui/src/main/java/jadx/gui/device/debugger/DebugController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

import javax.swing.JOptionPane;
import javax.swing.tree.DefaultMutableTreeNode;

import org.slf4j.Logger;
Expand Down Expand Up @@ -40,6 +41,7 @@
import jadx.gui.ui.panel.JDebuggerPanel;
import jadx.gui.ui.panel.JDebuggerPanel.IListElement;
import jadx.gui.ui.panel.JDebuggerPanel.ValueTreeNode;
import jadx.gui.utils.NLS;

public final class DebugController implements SmaliDebugger.SuspendListener, IDebugController {
private static final Logger LOG = LoggerFactory.getLogger(DebugController.class);
Expand Down Expand Up @@ -69,23 +71,22 @@ public final class DebugController implements SmaliDebugger.SuspendListener, IDe
private final ExecutorService updateQueue = Executors.newSingleThreadExecutor();
private final ExecutorService lazyQueue = Executors.newSingleThreadExecutor();

/**
* @param args at least 3 elements, host, port and android release version respectively.
*/
@Override
public boolean startDebugger(JDebuggerPanel debuggerPanel, String[] args) {
public boolean startDebugger(JDebuggerPanel debuggerPanel, String adbHost, int adbPort, int androidVer) {
if (TYPE_MAP.isEmpty()) {
initTypeMap();
}
this.debuggerPanel = debuggerPanel;
debuggerPanel.resetUI();
try {
debugger = SmaliDebugger.attach(args[0], Integer.parseInt(args[1]), this);
debugger = SmaliDebugger.attach(adbHost, adbPort, this);
} catch (SmaliDebuggerException e) {
JOptionPane.showMessageDialog(debuggerPanel.getMainWindow(), e.getMessage(),
NLS.str("error_dialog.title"), JOptionPane.ERROR_MESSAGE);
logErr(e);
return false;
}
art = ArtAdapter.getAdapter(Integer.parseInt(args[2]));
art = ArtAdapter.getAdapter(androidVer);
resetAllInfo();
hasResumed = false;
run = debugger::resume;
Expand Down
140 changes: 82 additions & 58 deletions jadx-gui/src/main/java/jadx/gui/device/protocol/ADB.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.io.InputStream;
import java.io.OutputStream;
import java.net.Socket;
import java.net.SocketException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand All @@ -14,10 +15,12 @@
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import jadx.core.utils.log.LogUtils;
import jadx.gui.utils.IOUtils;

public class ADB {
Expand All @@ -29,22 +32,36 @@ public class ADB {
private static final String CMD_FEATURES = "000dhost:features";
private static final String CMD_TRACK_DEVICES = "0014host:track-devices-l";
private static final byte[] OKAY = "OKAY".getBytes();
private static final byte[] FAIL = "FAIL".getBytes();

static boolean isOkay(InputStream stream) throws IOException {
byte[] buf = IOUtils.readNBytes(stream, 4);
return Arrays.equals(buf, OKAY);
if (Arrays.equals(buf, OKAY)) {
return true;
}
if (Arrays.equals(buf, FAIL)) {
// Observed that after FAIL the length in hex follows and afterwards an error message,
// but it is unclear if this is true for all cases where isOkay is used.
// int msgLen = Integer.parseInt(new String(IOUtils.readNBytes(stream, 4)), 16);
// byte[] errorMsg = IOUtils.readNBytes(stream, msgLen);
// LOG.error("isOkay failed: received error message: {}", new String(errorMsg));
LOG.error("isOkay failed");
return false;
}
if (buf == null) {
throw new IOException("isOkay failed - steam ended");
}
throw new IOException("isOkay failed - unexpected response " + new String(buf));
}

public static byte[] exec(String cmd, OutputStream outputStream, InputStream inputStream) throws IOException {
return execCommandSync(outputStream, inputStream, cmd);
}

public static byte[] exec(String cmd) throws IOException {
byte[] res;
try (Socket socket = connect()) {
res = exec(cmd, socket.getOutputStream(), socket.getInputStream());
return exec(cmd, socket.getOutputStream(), socket.getInputStream());
}
return res;
}

public static Socket connect() throws IOException {
Expand All @@ -55,14 +72,12 @@ public static Socket connect(String host, int port) throws IOException {
return new Socket(host, port);
}

static boolean execCommandAsync(OutputStream outputStream,
InputStream inputStream, String cmd) throws IOException {
static boolean execCommandAsync(OutputStream outputStream, InputStream inputStream, String cmd) throws IOException {
outputStream.write(cmd.getBytes());
return isOkay(inputStream);
}

private static byte[] execCommandSync(OutputStream outputStream,
InputStream inputStream, String cmd) throws IOException {
private static byte[] execCommandSync(OutputStream outputStream, InputStream inputStream, String cmd) throws IOException {
outputStream.write(cmd.getBytes());
if (isOkay(inputStream)) {
return readServiceProtocol(inputStream);
Expand All @@ -73,15 +88,26 @@ private static byte[] execCommandSync(OutputStream outputStream,
static byte[] readServiceProtocol(InputStream stream) {
try {
byte[] buf = IOUtils.readNBytes(stream, 4);
if (buf == null) {
return null;
}
int len = unhex(buf);
byte[] result;
if (len == 0) {
return new byte[0];
result = new byte[0];
} else {
result = IOUtils.readNBytes(stream, len);
}
if (LOG.isTraceEnabled()) {
LOG.trace("readServiceProtocol result: {}", LogUtils.escape(result));
}
return IOUtils.readNBytes(stream, len);
return result;
} catch (SocketException e) {
LOG.error("Aborting readServiceProtocol: socket closed");
} catch (IOException e) {
LOG.error("Failed to read readServiceProtocol: {}", e.toString());
return null;
LOG.error("Failed to read readServiceProtocol", e);
}
return null;
}

static boolean setSerial(String serial, OutputStream outputStream, InputStream inputStream) throws IOException {
Expand All @@ -91,7 +117,9 @@ static boolean setSerial(String serial, OutputStream outputStream, InputStream i
boolean ok = isOkay(inputStream);
if (ok) {
// skip the shell-state-id returned by ADB server, it's not important for the following actions.
ok = inputStream.skip(8) == 8;
IOUtils.readNBytes(inputStream, 8);
} else {
LOG.error("setSerial command {} failed", LogUtils.escape(setSerialCmd));
}
return ok;
}
Expand All @@ -106,8 +134,7 @@ private static byte[] execShellCommandRaw(String cmd, OutputStream outputStream,
return null;
}

static byte[] execShellCommandRaw(String serial, String cmd,
OutputStream outputStream, InputStream inputStream) throws IOException {
static byte[] execShellCommandRaw(String serial, String cmd, OutputStream outputStream, InputStream inputStream) throws IOException {
if (setSerial(serial, outputStream, inputStream)) {
return execShellCommandRaw(cmd, outputStream, inputStream);
}
Expand All @@ -124,14 +151,16 @@ public static List<String> getFeatures() throws IOException {

public static boolean startServer(String adbPath, int port) throws IOException {
String tcpPort = String.format("tcp:%d", port);
java.lang.Process proc = new ProcessBuilder(adbPath, "-L", tcpPort, "start-server")
List<String> command = Arrays.asList(adbPath, "-L", tcpPort, "start-server");
java.lang.Process proc = new ProcessBuilder(command)
.redirectErrorStream(true)
.start();
try {
proc.waitFor(3, TimeUnit.SECONDS); // for listening to a port, 3 sec should be more than enough.
// Wait for the adb server to start. On Windows even on a fast system 6 seconds are not unusual.
proc.waitFor(10, TimeUnit.SECONDS);
proc.exitValue();
} catch (Exception e) {
LOG.error("Start server error", e);
LOG.error("ADB start server failed with command: {}", String.join(" ", command), e);
proc.destroyForcibly();
return false;
}
Expand All @@ -143,7 +172,7 @@ public static boolean startServer(String adbPath, int port) throws IOException {
out.write(buf, 0, read);
}
}
return new String(out.toByteArray()).contains(tcpPort);
return out.toString().contains(tcpPort);
}

public static boolean isServerRunning(String host, int port) {
Expand All @@ -167,22 +196,21 @@ public static Socket listenForDeviceState(DeviceStateListener listener, String h
}
ExecutorService listenThread = Executors.newFixedThreadPool(1);
listenThread.execute(() -> {
for (;;) {
while (true) {
byte[] res = readServiceProtocol(inputStream);
if (res != null) {
if (listener != null) {
String payload = new String(res);
String[] deviceLines = payload.split("\n");
List<ADBDeviceInfo> deviceInfoList = new ArrayList<>(deviceLines.length);
for (String deviceLine : deviceLines) {
if (!deviceLine.trim().isEmpty()) {
deviceInfoList.add(ADBDeviceInfo.make(deviceLine, host, port));
}
if (res == null) {
break; // socket disconnected
}
if (listener != null) {
String payload = new String(res);
String[] deviceLines = payload.split("\n");
List<ADBDeviceInfo> deviceInfoList = new ArrayList<>(deviceLines.length);
for (String deviceLine : deviceLines) {
if (!deviceLine.trim().isEmpty()) {
deviceInfoList.add(ADBDeviceInfo.make(deviceLine, host, port));
}
listener.onDeviceStatusChange(deviceInfoList);
}
} else { // socket disconnected
break;
listener.onDeviceStatusChange(deviceInfoList);
}
}
if (listener != null) {
Expand All @@ -193,43 +221,39 @@ public static Socket listenForDeviceState(DeviceStateListener listener, String h
}

public static List<String> listForward(String host, int port) throws IOException {
Socket socket = connect(host, port);
String cmd = "0011host:list-forward";
InputStream inputStream = socket.getInputStream();
OutputStream outputStream = socket.getOutputStream();
outputStream.write(cmd.getBytes());
if (isOkay(inputStream)) {
byte[] bytes = readServiceProtocol(inputStream);
if (bytes != null) {
String[] forwards = new String(bytes).split("\n");
List<String> forwardList = Arrays.stream(forwards).map(s -> s.trim()).collect(Collectors.toList());
socket.close();
return forwardList;
try (Socket socket = connect(host, port)) {
String cmd = "0011host:list-forward";
InputStream inputStream = socket.getInputStream();
OutputStream outputStream = socket.getOutputStream();
outputStream.write(cmd.getBytes());
if (isOkay(inputStream)) {
byte[] bytes = readServiceProtocol(inputStream);
if (bytes != null) {
String[] forwards = new String(bytes).split("\n");
return Stream.of(forwards).map(String::trim).collect(Collectors.toList());
}
}
}
socket.close();
return Collections.emptyList();
}

public static boolean removeForward(String host, int port, String serial, String localPort) throws IOException {
Socket socket = connect(host, port);
String cmd = String.format("host:killforward:tcp:%s", localPort);
cmd = String.format("%04x%s", cmd.length(), cmd);
InputStream inputStream = socket.getInputStream();
OutputStream outputStream = socket.getOutputStream();
boolean ok = false;
if (setSerial(serial, outputStream, inputStream)) {
outputStream.write(cmd.getBytes());
ok = isOkay(inputStream) && isOkay(inputStream);
try (Socket socket = connect(host, port)) {
String cmd = String.format("host:killforward:tcp:%s", localPort);
cmd = String.format("%04x%s", cmd.length(), cmd);
InputStream inputStream = socket.getInputStream();
OutputStream outputStream = socket.getOutputStream();
if (setSerial(serial, outputStream, inputStream)) {
outputStream.write(cmd.getBytes());
return isOkay(inputStream) && isOkay(inputStream);
}
}
socket.close();
return ok;
return false;
}

// Little endian
private static int readInt(byte[] bytes, int start) {
int result = 0;
result = (bytes[start] & 0xff);
int result = (bytes[start] & 0xff);
result += ((bytes[start + 1] & 0xff) << 8);
result += ((bytes[start + 2] & 0xff) << 16);
result += (bytes[start + 3] & 0xff) << 24;
Expand Down
Loading

0 comments on commit 909cf0a

Please sign in to comment.