appdevforall · luisguzman-adfa · Jun 18, 2026 · Jun 18, 2026
diff --git a/CLAUDE.md b/CLAUDE.md
@@ -144,6 +144,21 @@ app's ABI, so a 32-bit app on a 64-bit phone reported 32-bit).
   use case; `getTermuxArch()` (app/content ABI) is unchanged for modules, termux
   and debian arch.
 
+**Slice (DONE) — sync credential validation (`org.iiab.controller.sync.domain`)**
+First Phase-1 *security* slice (refactor-by-feature while closing tech-debt **S1**).
+The peer-to-peer sync handshake carries host/port/user/pass in a scanned QR code;
+those values were interpolated into `rsyncd.conf` and a `rsync://` URL with no
+validation, allowing config-directive/URL injection.
+
+- `domain/` — `SyncCredentialValidator` (pure JVM, no Android): the single rule
+  for "what is a safe sync credential" — strict charsets for user/host, range
+  check for port, control-char-free password, and an `isSafeConfigValue` guard
+  for `rsyncd.conf` values. Unit-tested (`sync/domain/SyncCredentialValidatorTest`).
+- **Legacy seam:** `SyncHandshakeHelper.parsePayload()` now validates at the QR
+  parse boundary (returns `null` -> existing "invalid QR" toast), and
+  `RsyncManager` re-checks defensively before writing the daemon config / building
+  the client URL. Reusable by the remaining injection fixes (S4, D2) as they migrate.
+
 **Legacy (NOT yet layered)** — most of `org.iiab.controller` is still flat:
 god classes `MainActivity` and `DeployFragment` (~2.7k LOC), shared mutable
 state on public/static fields, hand-rolled `HttpURLConnection` calls duplicated

diff --git a/controller/app/src/main/java/org/iiab/controller/RsyncManager.java b/controller/app/src/main/java/org/iiab/controller/RsyncManager.java
@@ -14,6 +14,8 @@
 import android.os.Looper;
 import android.util.Log;
 
+import org.iiab.controller.sync.domain.SyncCredentialValidator;
+
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileOutputStream;
@@ -42,6 +44,18 @@ public boolean startServer(Context context, int port, String user, String pass,
         stop();
         isCancelled = false;
 
+        // S1 (defence in depth): never write attacker-controllable text into
+        // rsyncd.conf without validation. user/pass are app-generated and
+        // dirToShare is app-controlled, but a stray CR/LF here would let new
+        // config directives or module sections be injected.
+        if (!SyncCredentialValidator.isValidUsername(user)
+                || !SyncCredentialValidator.isValidPassword(pass)
+                || !SyncCredentialValidator.isValidPort(port)
+                || !SyncCredentialValidator.isSafeConfigValue(dirToShare)) {
+            Log.e(TAG, "Refusing to start rsync daemon: invalid credentials or share path");
+            return false;
+        }
+
         try {
             File nativeLibDir = new File(context.getApplicationInfo().nativeLibraryDir);
             File rsyncBin = new File(nativeLibDir, "librsync.so");
@@ -122,6 +136,13 @@ public void startClient(Context context, String hostIp, int port, String user, S
                     throw new Exception(context.getString(R.string.rsync_error_binary_missing));
                 }
 
+                // S1: reject credentials that could break out of the rsync:// URL.
+                if (!SyncCredentialValidator.validateCredentials(hostIp, port, user, pass).valid) {
+                    mainHandler.post(() -> listener.onError(
+                            context.getString(R.string.rsync_error_invalid_credentials)));
+                    return;
+                }
+
                 File passFile = new File(context.getCacheDir(), "rsync_client.pass");
                 writeTextToFile(passFile, pass);
 
@@ -220,6 +241,13 @@ public void calculateTransferPlan(Context context, String hostIp, int port, Stri
                 if (!rsyncBin.exists())
                     throw new Exception(context.getString(R.string.rsync_error_binary_missing));
 
+                // S1: reject credentials that could break out of the rsync:// URL.
+                if (!SyncCredentialValidator.validateCredentials(hostIp, port, user, pass).valid) {
+                    mainHandler.post(() -> listener.onError(
+                            context.getString(R.string.rsync_error_invalid_credentials)));
+                    return;
+                }
+
                 File passFile = new File(context.getCacheDir(), "rsync_client.pass");
                 writeTextToFile(passFile, pass);
                 passFile.setExecutable(false, false);

diff --git a/controller/app/src/main/java/org/iiab/controller/SyncHandshakeHelper.java b/controller/app/src/main/java/org/iiab/controller/SyncHandshakeHelper.java
@@ -14,6 +14,8 @@
 
 import org.json.JSONObject;
 
+import org.iiab.controller.sync.domain.SyncCredentialValidator;
+
 import java.security.SecureRandom;
 
 import com.google.zxing.BarcodeFormat;
@@ -76,11 +78,26 @@ public static SyncCredentials parsePayload(String scannedJson) {
                 Log.w(TAG, "Scanned QR is not an IIAB Sync code.");
                 return null;
             }
+            String ip = json.getString("ip");
+            int port = json.getInt("port");
+            String user = json.getString("user");
+            String pass = json.getString("pass");
+
+            // S1: the QR payload is untrusted input that is later interpolated
+            // into rsyncd.conf and a rsync:// URL. Reject anything that could
+            // inject config directives or break out of the URL.
+            SyncCredentialValidator.Result check =
+                    SyncCredentialValidator.validateCredentials(ip, port, user, pass);
+            if (!check.valid) {
+                Log.w(TAG, "Rejecting scanned credentials: " + check.reason);
+                return null;
+            }
+
             return new SyncCredentials(
-                    json.getString("ip"),
-                    json.getInt("port"),
-                    json.getString("user"),
-                    json.getString("pass"),
+                    ip,
+                    port,
+                    user,
+                    pass,
                     json.optBoolean("has_rootfs", true), // Default to true if missing for legacy compatibility
                     json.optInt("a", 0)
             );

diff --git a/controller/app/src/main/java/org/iiab/controller/sync/domain/SyncCredentialValidator.java b/controller/app/src/main/java/org/iiab/controller/sync/domain/SyncCredentialValidator.java
@@ -0,0 +1,184 @@
+/*
+ * ============================================================================
+ * Name        : SyncCredentialValidator.java
+ * Author      : IIAB Project
+ * Copyright   : Copyright (c) 2026 IIAB Project
+ * Description : Domain rule: what counts as a *safe* peer-to-peer sync
+ *               credential. Closes tech-debt item S1 (rsyncd.conf / rsync://
+ *               URL injection from QR-scanned credentials).
+ * ============================================================================
+ */
+package org.iiab.controller.sync.domain;
+
+/**
+ * Pure (framework-free) validation of peer-to-peer sync credentials.
+ *
+ * <p>The sync handshake transports a host IP, port, username and password
+ * inside a QR code that the receiving device scans. Those values are
+ * <strong>untrusted input</strong>: they are later interpolated into an
+ * {@code rsyncd.conf} file (server side) and into a {@code rsync://user@host:port/...}
+ * URL (client side). Without validation, a crafted QR code could inject extra
+ * {@code rsyncd.conf} directives (e.g. a newline followed by a new module
+ * section) or break out of the URL — see tech-debt item <b>S1</b>.
+ *
+ * <p>This class is the single source of truth for "what is a valid credential".
+ * It lives in the domain layer: no {@code android.*}, no networking, no I/O, so
+ * it is fully unit-testable on a plain JVM and reusable by any other injection
+ * fix (e.g. S4/D2) that needs to validate or quote shell/config input.
+ *
+ * <p>The policy is intentionally strict and fail-closed. Legitimate credentials
+ * are produced by {@code SyncHandshakeHelper.generateSecurePassword()}
+ * (12 alphanumeric chars) and a fixed username, so a conservative character set
+ * rejects attacks without rejecting any real value.
+ */
+public final class SyncCredentialValidator {
+
+    /** Max username length accepted in {@code auth users} / URL userinfo. */
+    private static final int MAX_USERNAME_LEN = 64;
+    /** Max password length accepted in the rsync password file. */
+    private static final int MAX_PASSWORD_LEN = 128;
+    /** Max host/IP length (DNS label limit is 253). */
+    private static final int MAX_HOST_LEN = 253;
+
+    private SyncCredentialValidator() {
+        // Static utility; not instantiable.
+    }
+
+    /** Outcome of a validation, with a machine-readable reason on failure. */
+    public static final class Result {
+        public final boolean valid;
+        /** Null when {@link #valid}; otherwise a short, log-safe reason. */
+        public final String reason;
+
+        private Result(boolean valid, String reason) {
+            this.valid = valid;
+            this.reason = reason;
+        }
+
+        public static Result ok() {
+            return new Result(true, null);
+        }
+
+        public static Result fail(String reason) {
+            return new Result(false, reason);
+        }
+    }
+
+    /**
+     * Validate a full set of scanned sync credentials. Call this at the parse
+     * boundary (QR decode); reject the whole handshake if it returns invalid.
+     */
+    public static Result validateCredentials(String ip, int port, String user, String pass) {
+        if (!isValidHost(ip)) {
+            return Result.fail("invalid host/ip");
+        }
+        if (!isValidPort(port)) {
+            return Result.fail("invalid port");
+        }
+        if (!isValidUsername(user)) {
+            return Result.fail("invalid username");
+        }
+        if (!isValidPassword(pass)) {
+            return Result.fail("invalid password");
+        }
+        return Result.ok();
+    }
+
+    /**
+     * A username safe for both the {@code rsync://USER@host} URL userinfo and
+     * the {@code auth users = USER} directive: a non-empty run of
+     * {@code [A-Za-z0-9_-]} starting with a letter, digit or underscore.
+     * Matches the fixed {@code iiab_peer} account.
+     */
+    public static boolean isValidUsername(String user) {
+        if (user == null || user.isEmpty() || user.length() > MAX_USERNAME_LEN) {
+            return false;
+        }
+        for (int i = 0; i < user.length(); i++) {
+            char c = user.charAt(i);
+            boolean ok = (c >= 'A' && c <= 'Z')
+                    || (c >= 'a' && c <= 'z')
+                    || (c >= '0' && c <= '9')
+                    || c == '_'
+                    || (c == '-' && i > 0); // '-' allowed, but not as the first char
+            if (!ok) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    /**
+     * A password safe to write as a single line in the rsync password / secrets
+     * file: printable ASCII only, no whitespace and no control characters (so it
+     * cannot add a second secrets line or a stray {@code rsyncd.conf} directive).
+     * The app's generated alphanumeric passwords satisfy this.
+     */
+    public static boolean isValidPassword(String pass) {
+        if (pass == null || pass.isEmpty() || pass.length() > MAX_PASSWORD_LEN) {
+            return false;
+        }
+        for (int i = 0; i < pass.length(); i++) {
+            char c = pass.charAt(i);
+            // Printable ASCII excluding space (0x20) and DEL (0x7f).
+            if (c <= 0x20 || c >= 0x7f) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    /**
+     * A host that is either a dotted IPv4 literal or a DNS hostname, using only
+     * {@code [A-Za-z0-9.-]}. Rejects anything containing {@code @ : / \\},
+     * whitespace or control characters that could break out of the URL.
+     */
+    public static boolean isValidHost(String host) {
+        if (host == null || host.isEmpty() || host.length() > MAX_HOST_LEN) {
+            return false;
+        }
+        for (int i = 0; i < host.length(); i++) {
+            char c = host.charAt(i);
+            boolean ok = (c >= 'A' && c <= 'Z')
+                    || (c >= 'a' && c <= 'z')
+                    || (c >= '0' && c <= '9')
+                    || c == '.'
+                    || c == '-';
+            if (!ok) {
+                return false;
+            }
+        }
+        // Reject leading/trailing dot or dash and empty labels (".." / "a..b").
+        if (host.charAt(0) == '.' || host.charAt(0) == '-'
+                || host.charAt(host.length() - 1) == '.'
+                || host.charAt(host.length() - 1) == '-'
+                || host.contains("..")) {
+            return false;
+        }
+        return true;
+    }
+
+    /** TCP port in the valid 1..65535 range. */
+    public static boolean isValidPort(int port) {
+        return port >= 1 && port <= 65535;
+    }
+
+    /**
+     * True if {@code value} is safe to embed verbatim on a single line of an
+     * {@code rsyncd.conf} file: no control characters (notably CR/LF, which
+     * would let an attacker append new directives or module sections) and no
+     * NUL. Use this for app-controlled values such as the shared directory path.
+     */
+    public static boolean isSafeConfigValue(String value) {
+        if (value == null) {
+            return false;
+        }
+        for (int i = 0; i < value.length(); i++) {
+            char c = value.charAt(i);
+            if (c == '\n' || c == '\r' || c == '\0' || c < 0x20 && c != '\t') {
+                return false;
+            }
+        }
+        return true;
+    }
+}
diff --git a/controller/app/src/main/res/values-es/strings.xml b/controller/app/src/main/res/values-es/strings.xml
@@ -499,4 +499,5 @@
     <string name="sync_app_arch_label">App Arch: %1$d-bit</string>
     <string name="sync_msg_arch_compatible">Hardware compatible. Iniciando conexión...</string>
 
+    <string name="rsync_error_invalid_credentials">Credenciales de sincronización inválidas o no seguras. Escanea un código QR de sincronización IIAB válido.</string>
 </resources>
diff --git a/controller/app/src/main/res/values/strings.xml b/controller/app/src/main/res/values/strings.xml
@@ -514,4 +514,5 @@
     <string name="sync_app_arch_label">App Arch: %1$d-bit</string>
     <string name="sync_msg_arch_compatible">Hardware compatible. Starting connection...</string>
 
+    <string name="rsync_error_invalid_credentials">Invalid or unsafe sync credentials. Please scan a valid IIAB sync QR code.</string>
 </resources>
diff --git a/controller/app/src/test/java/org/iiab/controller/SyncHandshakeHelperTest.java b/controller/app/src/test/java/org/iiab/controller/SyncHandshakeHelperTest.java
@@ -79,4 +79,27 @@ public void parsePayload_defaultsHasRootfsTrueForLegacyPayload() {
         assertTrue(creds.hasRootfs);
         assertEquals(0, creds.archBits);
     }
+    // --- S1: malicious payloads are rejected at the parse boundary ---
+
+    @Test
+    public void parsePayload_rejectsRsyncdConfInjectionInUsername() {
+        // A username carrying a newline + a new rsyncd.conf section.
+        String malicious = "{\"app\":\"iiab_sync\",\"ip\":\"10.0.0.1\",\"port\":8730,"
+                + "\"user\":\"iiab\\n[evil]\\npath = /\",\"pass\":\"p\"}";
+        assertNull(SyncHandshakeHelper.parsePayload(malicious));
+    }
+
+    @Test
+    public void parsePayload_rejectsUrlBreakoutInHost() {
+        String malicious = "{\"app\":\"iiab_sync\",\"ip\":\"10.0.0.1/evil\",\"port\":8730,"
+                + "\"user\":\"iiab_peer\",\"pass\":\"p\"}";
+        assertNull(SyncHandshakeHelper.parsePayload(malicious));
+    }
+
+    @Test
+    public void parsePayload_rejectsOutOfRangePort() {
+        String malicious = "{\"app\":\"iiab_sync\",\"ip\":\"10.0.0.1\",\"port\":70000,"
+                + "\"user\":\"iiab_peer\",\"pass\":\"p\"}";
+        assertNull(SyncHandshakeHelper.parsePayload(malicious));
+    }
 }