security: fix all review findings, bump to 1.0.19 (build 20)

CRITICAL - SftpProvider: replace PromiscuousVerifier with TofuHostKeyVerifier (trust-on-first-use; stores SHA-256 fingerprints in EncryptedSharedPreferences; rejects key changes on subsequent connections) HIGH - GoogleDriveProvider: replace raw string interpolation with buildJsonObject in uploadFile, createDirectory, and moveFile to prevent JSON injection - DropboxProvider: replace all raw JSON strings and Dropbox-API-Arg headers with buildJsonObject for the same reason - OAuthHelper: add cryptographically random state parameter to Dropbox and OneDrive authorization URLs (stored alongside the PKCE verifier) - OAuthRedirectActivity: validate returned state against stored value before exchanging the authorization code (CSRF protection) MEDIUM - WebDavProvider: block cross-host redirects in the manual redirect interceptor so Authorization headers are never forwarded to a different server - AccountSetupScreen: set FLAG_SECURE on the window while credential fields are visible to prevent screenshots and screen-recording capture - libs.versions.toml: security-crypto alpha06 → stable 1.0.0; biometric-ktx alpha05 → biometric 1.1.0 (stable, non-ktx artifact matches the BiometricManager/BiometricPrompt API actually used in MainActivity) - CredentialStore: migrate to security-crypto 1.0.0 API (MasterKeys.getOrCreate + positional create() args); add saveHostKey/getHostFingerprint for SFTP TOFU Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-24 18:08:40 +00:00
parent 894c2ffe78
commit be3f46287a
12 changed files with 128 additions and 38 deletions
@@ -7,19 +7,20 @@ import com.syncflow.data.providers.owncloud.OwnCloudProvider
 import com.syncflow.data.providers.onedrive.OneDriveProvider
 import com.syncflow.data.providers.sftp.SftpProvider
 import com.syncflow.data.providers.webdav.WebDavProvider
+import com.syncflow.data.security.CredentialStore
 import com.syncflow.domain.model.CloudAccount
 import com.syncflow.domain.model.ProviderType
 import javax.inject.Inject
 import javax.inject.Singleton

@Singleton
-class ProviderFactory @Inject constructor() {
+class ProviderFactory @Inject constructor(private val credentialStore: CredentialStore) {
    fun create(account: CloudAccount): CloudProvider = when (account.providerType) {
        ProviderType.GOOGLE_DRIVE -> GoogleDriveProvider(account)
        ProviderType.DROPBOX -> DropboxProvider(account)
        ProviderType.ONEDRIVE -> OneDriveProvider(account)
        ProviderType.WEBDAV -> WebDavProvider(account)
-        ProviderType.SFTP -> SftpProvider(account)
+        ProviderType.SFTP -> SftpProvider(account, credentialStore)
        ProviderType.NEXTCLOUD -> NextcloudProvider(account)
        ProviderType.OWNCLOUD -> OwnCloudProvider(account)
        ProviderType.SFTPGO -> WebDavProvider(account)     // SFTPGo exposes WebDAV
@@ -18,9 +18,9 @@ class DropboxProvider(private val account: CloudAccount) : CloudProvider {
    }
    private val client = OkHttpClient()

-    private fun apiReq(url: String, bodyJson: String): Request =
+    private fun apiReq(url: String, argJson: JsonObject): Request =
        Request.Builder().url(url)
-            .post(bodyJson.toRequestBody("application/json".toMediaType()))
+            .post(argJson.toString().toRequestBody("application/json".toMediaType()))
            .header("Authorization", "Bearer $token")
            .build()

@@ -33,7 +33,8 @@ class DropboxProvider(private val account: CloudAccount) : CloudProvider {

    override suspend fun listFiles(remotePath: String): Result<List<RemoteFile>> = runCatching {
        val path = if (remotePath == "/" || remotePath.isBlank()) "" else remotePath
-        val req = apiReq("https://api.dropboxapi.com/2/files/list_folder", """{"path":"$path","recursive":false}""")
+        val arg = buildJsonObject { put("path", path); put("recursive", false) }
+        val req = apiReq("https://api.dropboxapi.com/2/files/list_folder", arg)
        client.newCall(req).execute().use { resp ->
            val body = resp.body?.string() ?: throw Exception("HTTP ${resp.code}")
            if (!resp.isSuccessful) throw Exception("HTTP ${resp.code}: $body")
@@ -44,11 +45,15 @@ class DropboxProvider(private val account: CloudAccount) : CloudProvider {

    override suspend fun uploadFile(localStream: InputStream, remotePath: String, sizeBytes: Long, onProgress: (Long) -> Unit): Result<RemoteFile> = runCatching {
        val bytes = localStream.readBytes()
-        val argHeader = """{"path":"${remotePath.normalizeDropbox()}","mode":"overwrite","autorename":false}"""
+        val arg = buildJsonObject {
+            put("path", remotePath.normalizeDropbox())
+            put("mode", "overwrite")
+            put("autorename", false)
+        }
        val req = Request.Builder().url("https://content.dropboxapi.com/2/files/upload")
            .post(bytes.toRequestBody("application/octet-stream".toMediaType()))
            .header("Authorization", "Bearer $token")
-            .header("Dropbox-API-Arg", argHeader).build()
+            .header("Dropbox-API-Arg", arg.toString()).build()
        client.newCall(req).execute().use { resp ->
            val body = resp.body?.string() ?: throw Exception("HTTP ${resp.code}")
            if (!resp.isSuccessful) throw Exception("HTTP ${resp.code}: $body")
@@ -58,11 +63,11 @@ class DropboxProvider(private val account: CloudAccount) : CloudProvider {
    }

    override suspend fun downloadFile(remotePath: String, destination: OutputStream, onProgress: (Long) -> Unit): Result<Unit> = runCatching {
-        val argHeader = """{"path":"${remotePath.normalizeDropbox()}"}"""
+        val arg = buildJsonObject { put("path", remotePath.normalizeDropbox()) }
        val req = Request.Builder().url("https://content.dropboxapi.com/2/files/download")
            .post("".toRequestBody())
            .header("Authorization", "Bearer $token")
-            .header("Dropbox-API-Arg", argHeader).build()
+            .header("Dropbox-API-Arg", arg.toString()).build()
        client.newCall(req).execute().use { resp ->
            if (!resp.isSuccessful) throw Exception("HTTP ${resp.code}")
            var total = 0L
@@ -75,17 +80,20 @@ class DropboxProvider(private val account: CloudAccount) : CloudProvider {
    }

    override suspend fun deleteFile(remotePath: String): Result<Unit> = runCatching {
-        val req = apiReq("https://api.dropboxapi.com/2/files/delete_v2", """{"path":"${remotePath.normalizeDropbox()}"}""")
+        val arg = buildJsonObject { put("path", remotePath.normalizeDropbox()) }
+        val req = apiReq("https://api.dropboxapi.com/2/files/delete_v2", arg)
        client.newCall(req).execute().use { resp -> if (!resp.isSuccessful) throw Exception("HTTP ${resp.code}") }
    }

    override suspend fun createDirectory(remotePath: String): Result<Unit> = runCatching {
-        val req = apiReq("https://api.dropboxapi.com/2/files/create_folder_v2", """{"path":"${remotePath.normalizeDropbox()}"}""")
+        val arg = buildJsonObject { put("path", remotePath.normalizeDropbox()) }
+        val req = apiReq("https://api.dropboxapi.com/2/files/create_folder_v2", arg)
        client.newCall(req).execute().use { resp -> if (!resp.isSuccessful && resp.code != 409) throw Exception("HTTP ${resp.code}") }
    }

    override suspend fun getFileMetadata(remotePath: String): Result<RemoteFile> = runCatching {
-        val req = apiReq("https://api.dropboxapi.com/2/files/get_metadata", """{"path":"${remotePath.normalizeDropbox()}"}""")
+        val arg = buildJsonObject { put("path", remotePath.normalizeDropbox()) }
+        val req = apiReq("https://api.dropboxapi.com/2/files/get_metadata", arg)
        client.newCall(req).execute().use { resp ->
            val body = resp.body?.string() ?: throw Exception("HTTP ${resp.code}")
            Json.parseToJsonElement(body).jsonObject.toRemoteFile()
@@ -93,8 +101,11 @@ class DropboxProvider(private val account: CloudAccount) : CloudProvider {
    }

    override suspend fun moveFile(fromPath: String, toPath: String): Result<Unit> = runCatching {
-        val req = apiReq("https://api.dropboxapi.com/2/files/move_v2",
-            """{"from_path":"${fromPath.normalizeDropbox()}","to_path":"${toPath.normalizeDropbox()}"}""")
+        val arg = buildJsonObject {
+            put("from_path", fromPath.normalizeDropbox())
+            put("to_path", toPath.normalizeDropbox())
+        }
+        val req = apiReq("https://api.dropboxapi.com/2/files/move_v2", arg)
        client.newCall(req).execute().use { resp -> if (!resp.isSuccessful) throw Exception("HTTP ${resp.code}") }
    }

@@ -44,9 +44,11 @@ class GoogleDriveProvider(private val account: CloudAccount) : CloudProvider {
        val name = remotePath.substringAfterLast('/')
        val parentId = if ('/' in remotePath.dropLast(1)) remotePath.substringBeforeLast('/') else "root"

-        // Multipart upload
-        val metaPart = """{"name":"$name","parents":["$parentId"]}"""
-            .toRequestBody("application/json".toMediaType())
+        // Multipart upload — use JSON builder to avoid injection via filenames with special chars
+        val metaPart = buildJsonObject {
+            put("name", name)
+            put("parents", buildJsonArray { add(parentId) })
+        }.toString().toRequestBody("application/json".toMediaType())
        val dataPart = bytes.toRequestBody("application/octet-stream".toMediaType())
        val multipart = MultipartBody.Builder()
            .setType(MultipartBody.FORM)
@@ -86,8 +88,11 @@ class GoogleDriveProvider(private val account: CloudAccount) : CloudProvider {
    override suspend fun createDirectory(remotePath: String): Result<Unit> = runCatching {
        val name = remotePath.substringAfterLast('/')
        val parentId = if ('/' in remotePath.dropLast(1)) remotePath.substringBeforeLast('/') else "root"
-        val body = """{"name":"$name","mimeType":"application/vnd.google-apps.folder","parents":["$parentId"]}"""
-            .toRequestBody("application/json".toMediaType())
+        val body = buildJsonObject {
+            put("name", name)
+            put("mimeType", "application/vnd.google-apps.folder")
+            put("parents", buildJsonArray { add(parentId) })
+        }.toString().toRequestBody("application/json".toMediaType())
        val req = auth(Request.Builder().url("https://www.googleapis.com/drive/v3/files").post(body)).build()
        client.newCall(req).execute().use { resp -> if (!resp.isSuccessful) throw Exception("HTTP ${resp.code}") }
    }
@@ -102,7 +107,8 @@ class GoogleDriveProvider(private val account: CloudAccount) : CloudProvider {

    override suspend fun moveFile(fromPath: String, toPath: String): Result<Unit> = runCatching {
        val newName = toPath.substringAfterLast('/')
-        val body = """{"name":"$newName"}""".toRequestBody("application/json".toMediaType())
+        val body = buildJsonObject { put("name", newName) }.toString()
+            .toRequestBody("application/json".toMediaType())
        val req = auth(Request.Builder().url("https://www.googleapis.com/drive/v3/files/$fromPath").patch(body)).build()
        client.newCall(req).execute().use { resp -> if (!resp.isSuccessful) throw Exception("HTTP ${resp.code}") }
    }
@@ -1,6 +1,7 @@
 package com.syncflow.data.providers.sftp

 import com.syncflow.data.providers.CloudProvider
+import com.syncflow.data.security.CredentialStore
 import com.syncflow.domain.model.CloudAccount
 import com.syncflow.domain.model.RemoteFile
 import kotlinx.serialization.json.Json
@@ -8,13 +9,12 @@ import kotlinx.serialization.json.jsonObject
 import kotlinx.serialization.json.jsonPrimitive
 import net.schmizz.sshj.SSHClient
 import net.schmizz.sshj.sftp.SFTPClient
-import net.schmizz.sshj.transport.verification.PromiscuousVerifier
 import net.schmizz.sshj.xfer.InMemorySourceFile
 import java.io.InputStream
 import java.io.OutputStream
 import java.time.Instant

-class SftpProvider(private val account: CloudAccount) : CloudProvider {
+class SftpProvider(private val account: CloudAccount, private val credentialStore: CredentialStore) : CloudProvider {

    private val creds = Json.parseToJsonElement(account.credentialJson).jsonObject
    private val host = account.serverUrl ?: "localhost"
@@ -25,7 +25,7 @@ class SftpProvider(private val account: CloudAccount) : CloudProvider {

    private fun <T> withSftp(block: (SFTPClient) -> T): T {
        val ssh = SSHClient()
-        ssh.addHostKeyVerifier(PromiscuousVerifier()) // TODO: replace with proper key pinning
+        ssh.addHostKeyVerifier(TofuHostKeyVerifier(credentialStore))
        ssh.connect(host, port)
        try {
            if (!privateKey.isNullOrBlank()) {
@@ -0,0 +1,31 @@
+package com.syncflow.data.providers.sftp
+
+import com.syncflow.data.security.CredentialStore
+import net.schmizz.sshj.transport.verification.HostKeyVerifier
+import java.security.MessageDigest
+import java.security.PublicKey
+
+/**
+ * Trust-On-First-Use SSH host key verifier.
+ *
+ * First connection to a host: fingerprint is stored in EncryptedSharedPreferences and accepted.
+ * Subsequent connections: stored fingerprint must match — mismatch aborts (possible MITM).
+ */
+class TofuHostKeyVerifier(private val credentialStore: CredentialStore) : HostKeyVerifier {
+
+    override fun verify(hostname: String, port: Int, key: PublicKey): Boolean {
+        val fingerprint = sha256Fingerprint(key)
+        val stored = credentialStore.getHostFingerprint(hostname, port)
+        return if (stored == null) {
+            credentialStore.saveHostKey(hostname, port, fingerprint)
+            true
+        } else {
+            stored == fingerprint
+        }
+    }
+
+    private fun sha256Fingerprint(key: PublicKey): String {
+        val digest = MessageDigest.getInstance("SHA-256").digest(key.encoded)
+        return digest.joinToString(":") { "%02x".format(it) }
+    }
+}
@@ -10,6 +10,7 @@ import kotlinx.serialization.json.Json
 import kotlinx.serialization.json.jsonObject
 import kotlinx.serialization.json.jsonPrimitive
 import okhttp3.*
+import okhttp3.HttpUrl.Companion.toHttpUrlOrNull
 import okhttp3.MediaType.Companion.toMediaType
 import okhttp3.RequestBody.Companion.toRequestBody
 import org.xmlpull.v1.XmlPullParser
@@ -38,9 +39,14 @@ open class WebDavProvider(protected val account: CloudAccount) : CloudProvider {
                    .header("Authorization", Credentials.basic(user, pass))
                    .build()
                val resp = chain.proceed(req)
-                // follow redirect manually for WebDAV methods (OkHttp skips non-GET/HEAD redirects)
+                // Follow redirects for WebDAV methods (OkHttp skips non-GET/HEAD redirects).
+                // Only follow same-host redirects to prevent credential leakage to a different server.
                if (resp.code in 301..308) {
                    val location = resp.header("Location") ?: return@addInterceptor resp
+                    val redirectHost = location.toHttpUrlOrNull()?.host
+                    if (redirectHost == null || redirectHost != req.url.host) {
+                        return@addInterceptor resp
+                    }
                    resp.close()
                    val redirectReq = req.newBuilder().url(location).build()
                    chain.proceed(redirectReq)
@@ -3,7 +3,7 @@ package com.syncflow.data.security
 import android.content.Context
 import android.content.SharedPreferences
 import androidx.security.crypto.EncryptedSharedPreferences
-import androidx.security.crypto.MasterKey
+import androidx.security.crypto.MasterKeys
 import dagger.hilt.android.qualifiers.ApplicationContext
 import javax.inject.Inject
 import javax.inject.Singleton
@@ -12,13 +12,11 @@ import javax.inject.Singleton
 class CredentialStore @Inject constructor(@ApplicationContext private val context: Context) {

    private val prefs: SharedPreferences by lazy {
-        val masterKey = MasterKey.Builder(context)
-            .setKeyScheme(MasterKey.KeyScheme.AES256_GCM)
-            .build()
+        val masterKeyAlias = MasterKeys.getOrCreate(MasterKeys.AES256_GCM_SPEC)
        EncryptedSharedPreferences.create(
-            context,
            "syncflow_credentials",
-            masterKey,
+            masterKeyAlias,
+            context,
            EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
            EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM,
        )
@@ -37,7 +35,7 @@ class CredentialStore @Inject constructor(@ApplicationContext private val contex
        prefs.edit().remove(credKey(accountId)).apply()
    }

-    // ── PKCE verifiers (OAuth flow) ───────────────────────────────────────────
+    // ── PKCE verifiers and OAuth state (OAuth flow) ───────────────────────────

    fun savePkceVerifier(provider: String, verifier: String) {
        prefs.edit().putString(pkceKey(provider), verifier).apply()
@@ -49,8 +47,18 @@ class CredentialStore @Inject constructor(@ApplicationContext private val contex
        prefs.edit().remove(pkceKey(provider)).apply()
    }

+    // ── SFTP host key fingerprints (TOFU) ─────────────────────────────────────
+
+    fun saveHostKey(host: String, port: Int, fingerprint: String) {
+        prefs.edit().putString(hostKey(host, port), fingerprint).apply()
+    }
+
+    fun getHostFingerprint(host: String, port: Int): String? =
+        prefs.getString(hostKey(host, port), null)
+
    // ── Key helpers ───────────────────────────────────────────────────────────

    private fun credKey(accountId: Long) = "cred_$accountId"
    private fun pkceKey(provider: String) = "pkce_$provider"
+    private fun hostKey(host: String, port: Int) = "sshhost_${host}_$port"
 }
@@ -2,6 +2,7 @@ package com.syncflow.ui.auth

 import android.accounts.AccountManager
 import android.app.Activity
+import android.view.WindowManager
 import androidx.activity.compose.rememberLauncherForActivityResult
 import androidx.activity.result.contract.ActivityResultContracts
 import androidx.compose.foundation.Image
@@ -201,6 +202,15 @@ private fun CredentialContent(
 ) {
    val provider = state.providerType ?: return

+    // Prevent screenshots and screen recording while credentials are visible
+    val activity = LocalContext.current as? Activity
+    DisposableEffect(Unit) {
+        activity?.window?.addFlags(WindowManager.LayoutParams.FLAG_SECURE)
+        onDispose {
+            activity?.window?.clearFlags(WindowManager.LayoutParams.FLAG_SECURE)
+        }
+    }
+
    Column(
        modifier = modifier
            .padding(horizontal = 20.dp)
@@ -38,7 +38,9 @@ private fun generateChallenge(verifier: String): String {

 fun launchDropboxOAuth(context: Context, credentialStore: CredentialStore, appKey: String) {
    val verifier = generateVerifier()
+    val state = generateVerifier()
    credentialStore.savePkceVerifier("dropbox", verifier)
+    credentialStore.savePkceVerifier("dropbox_state", state)
    val challenge = generateChallenge(verifier)
    val url = "https://www.dropbox.com/oauth2/authorize" +
        "?client_id=$appKey" +
@@ -46,13 +48,16 @@ fun launchDropboxOAuth(context: Context, credentialStore: CredentialStore, appKe
        "&redirect_uri=syncflow%3A%2F%2Foauth%2Fdropbox" +
        "&code_challenge=$challenge" +
        "&code_challenge_method=S256" +
-        "&token_access_type=offline"
+        "&token_access_type=offline" +
+        "&state=$state"
    openCustomTab(context, url)
 }

 fun launchOneDriveOAuth(context: Context, credentialStore: CredentialStore, clientId: String) {
    val verifier = generateVerifier()
+    val state = generateVerifier()
    credentialStore.savePkceVerifier("onedrive", verifier)
+    credentialStore.savePkceVerifier("onedrive_state", state)
    val challenge = generateChallenge(verifier)
    val scopes = "Files.ReadWrite+User.Read+offline_access"
    val url = "https://login.microsoftonline.com/common/oauth2/v2.0/authorize" +
@@ -61,7 +66,8 @@ fun launchOneDriveOAuth(context: Context, credentialStore: CredentialStore, clie
        "&redirect_uri=syncflow%3A%2F%2Foauth%2Fonedrive" +
        "&scope=$scopes" +
        "&code_challenge=$challenge" +
-        "&code_challenge_method=S256"
+        "&code_challenge_method=S256" +
+        "&state=$state"
    openCustomTab(context, url)
 }

@@ -28,11 +28,22 @@ class OAuthRedirectActivity : ComponentActivity() {
    private fun handleIntent(intent: Intent) {
        val uri = intent.data ?: run { finish(); return }
        val code = uri.getQueryParameter("code") ?: run { finish(); return }
+        val returnedState = uri.getQueryParameter("state") ?: run { finish(); return }
+
        val provider = when {
            uri.host == "oauth" && uri.path?.contains("dropbox") == true -> "dropbox"
            uri.host == "oauth" && uri.path?.contains("onedrive") == true -> "onedrive"
            else -> run { finish(); return }
        }
+
+        // Validate state before doing anything with the code (CSRF protection)
+        val storedState = credentialStore.getPkceVerifier("${provider}_state")
+        if (storedState == null || returnedState != storedState) {
+            finish()
+            return
+        }
+        credentialStore.removePkceVerifier("${provider}_state")
+
        val appKey = getString(com.syncflow.R.string.dropbox_app_key)
        val odClientId = getString(com.syncflow.R.string.onedrive_client_id)
        lifecycleScope.launch {
@@ -28,8 +28,8 @@ localbroadcastmanager = "1.1.0"
 coil = "2.7.0"
 splashscreen = "1.0.1"
 timber = "5.0.1"
-securityCrypto = "1.1.0-alpha06"
-biometric = "1.2.0-alpha05"
+securityCrypto = "1.0.0"
+biometric = "1.1.0"
 junit = "4.13.2"
 androidxTestExt = "1.2.1"
 espresso = "3.6.1"
@@ -106,7 +106,7 @@ coil-compose = { group = "io.coil-kt", name = "coil-compose", version.ref = "coi

 # Security
 security-crypto = { group = "androidx.security", name = "security-crypto", version.ref = "securityCrypto" }
-biometric = { group = "androidx.biometric", name = "biometric-ktx", version.ref = "biometric" }
+biometric = { group = "androidx.biometric", name = "biometric", version.ref = "biometric" }

 # Logging
 timber = { group = "com.jakewharton.timber", name = "timber", version.ref = "timber" }
@@ -1,2 +1,2 @@
-VERSION_NAME=1.0.18
-VERSION_CODE=19
+VERSION_NAME=1.0.19
+VERSION_CODE=20