fix: keep every provider write of device sync inside its own account #34

The native side ANDs the account's scope onto each selection the engine sends, as `(where) AND (scope)`. A selection such as `1) OR (1` closed the scope's parenthesis itself: on the calendar tables, where CalendarProvider adds no account filter of its own, an update or delete built that way would have reached the events, attendees and calendars of other accounts, e.g. the user's Google calendar. The engine only sends constant selections, so this guards against bugs rather than anything an outside app could send, which is what the scope is for. - Selections and sort orders must balance their parentheses outside quoted literals, and may not end the statement, carry comments or query other tables (`SELECT`, `UNION`, `ATTACH`, `PRAGMA`). - An update of a data row by id that writes `data1` without restating the mimetype has the row's mimetype looked up: on a membership row `data1` names a group, which must be the account's, as on insert. Such writes by selection are refused. - `data14`, a photo row's file id, is never written: another account's file id would have let `readPhoto` read that account's photo. Checked: 58 JVM tests (`gradlew :app:testDebugUnitTest`, three new), the debug build, and every selection the engine and planners send against the new rules.
2026-11-02 00:06:23 +00:00 · 2026-09-27 06:12:08 +02:00
parent d135bb2449
commit 49c5d05c30
4 changed files with 177 additions and 6 deletions
@@ -199,6 +199,14 @@ class ProviderIo(context: Context, accountName: String, private val authority: S
    override fun dataRawContacts(ids: Set<Long>): Map<Long, Long> =
        idPairs(ProviderTable.DATA, "raw_contact_id", ids)

+    override fun dataMimetypes(ids: Set<Long>): Map<Long, String> {
+        val mimetypes = HashMap<Long, String>()
+        forEachRow(ProviderTable.DATA, arrayOf(ID, "mimetype"), ID, ids) { c ->
+            if (!c.isNull(1)) mimetypes[c.getLong(0)] = c.getString(1)
+        }
+        return mimetypes
+    }
+
    override fun eventCalendars(ids: Set<Long>): Map<Long, Long> =
        idPairs(ProviderTable.EVENTS, "calendar_id", ids)

@@ -158,6 +158,61 @@ sealed interface ProviderOp {

 const val GROUP_MEMBERSHIP_MIMETYPE = "vnd.android.cursor.item/group_membership"

+/**
+ * Checks a selection or sort order before the account's scope is ANDed onto
+ * it as `(text) AND (scope)`: its parentheses must balance outside quoted
+ * literals, or `1) OR (1` would close the scope's own parenthesis. It may
+ * not end or comment out the statement, nor query other tables. The engine's
+ * selections are constants with `?` arguments; this guards against bugs.
+ */
+object SqlFragments {
+    private val FORBIDDEN_WORDS = Regex("\\b(SELECT|UNION|ATTACH|DETACH|PRAGMA)\\b", RegexOption.IGNORE_CASE)
+
+    fun check(text: String, what: String) {
+        var depth = 0
+        var quote: Char? = null
+        val outside = StringBuilder(text.length)
+        var i = 0
+        while (i < text.length) {
+            val c = text[i]
+            if (quote != null) {
+                if (c == quote) {
+                    // A doubled quote is an escaped one inside the literal.
+                    if (i + 1 < text.length && text[i + 1] == quote) i++ else quote = null
+                }
+                outside.append(' ')
+            } else {
+                when (c) {
+                    '\'', '"' -> {
+                        quote = c
+                        outside.append(' ')
+                    }
+                    '(' -> {
+                        depth++
+                        outside.append(c)
+                    }
+                    ')' -> {
+                        if (--depth < 0) refuse(what, "closes a parenthesis it did not open")
+                        outside.append(c)
+                    }
+                    ';' -> refuse(what, "ends the statement")
+                    '\u0000' -> refuse(what, "contains a NUL character")
+                    else -> outside.append(c)
+                }
+            }
+            i++
+        }
+        if (quote != null) refuse(what, "leaves a quote open")
+        if (depth != 0) refuse(what, "leaves a parenthesis open")
+        val bare = outside.toString()
+        if ("--" in bare || "/*" in bare) refuse(what, "contains a comment")
+        FORBIDDEN_WORDS.find(bare)?.let { refuse(what, "uses ${it.value.uppercase()}") }
+    }
+
+    private fun refuse(what: String, why: String): Nothing =
+        throw ProviderRefusal(BatchFailure.SCOPE, "$what $why")
+}
+
 /**
 * The table a written column points into when the row it names must belong to
 * the account as well: the parent of a row, the master of an exception event,
@@ -191,7 +246,8 @@ object ProviderOpParser {
        }
        if (columns.isEmpty()) throw invalid("query needs at least one column")
        val (where, args) = o.whereArgs("query")
-        return ProviderQuery(table, columns, where, args, o.optionalString("orderBy"))
+        val orderBy = o.optionalString("orderBy")?.also { SqlFragments.check(it, "query: orderBy") }
+        return ProviderQuery(table, columns, where, args, orderBy)
    }

    fun parseBatch(authority: String, json: String): List<ProviderOp> {
@@ -323,7 +379,7 @@ object ProviderOpParser {
    }

    private fun JSONObject.whereArgs(where: String): Pair<String?, List<String>> {
-        val sql = optionalString("where")
+        val sql = optionalString("where")?.also { SqlFragments.check(it, "$where: where") }
        val args = when (val raw = opt("args")) {
            null, JSONObject.NULL -> emptyList()
            is JSONArray -> (0 until raw.length()).map { i ->
@@ -45,21 +45,45 @@ class ScopePlanner(private val account: AccountRef, private val lookup: ScopeLoo
    }

    fun planBatch(ops: List<ProviderOp>): List<PlannedOp> {
-        val owners = Owners(ownershipRequests(ops))
-        return ops.mapIndexed { i, op -> plan(i, op, owners) }
+        val memberships = membershipUpdates(ops)
+        val owners = Owners(ownershipRequests(ops, memberships))
+        return ops.mapIndexed { i, op -> plan(i, op, owners, memberships) }
    }

-    private fun plan(i: Int, op: ProviderOp, owners: Owners): PlannedOp = when (op) {
+    /**
+     * Membership rows among the data rows that updates by id write `data1`
+     * of without restating the mimetype: their `data1` names a group, which
+     * must be the account's like on insert.
+     */
+    private fun membershipUpdates(ops: List<ProviderOp>): Set<Long> {
+        val ids = ops.asSequence()
+            .filterIsInstance<ProviderOp.Update>()
+            .filter { it.table == ProviderTable.DATA && writesUntypedData1(it.values) }
+            .mapNotNull { it.target.id }
+            .toSet()
+        if (ids.isEmpty()) return emptySet()
+        return lookup.dataMimetypes(ids).filterValues { it == GROUP_MEMBERSHIP_MIMETYPE }.keys
+    }
+
+    private fun writesUntypedData1(values: Map<String, Cell>) =
+        MIMETYPE !in values && (values[DATA1] ?: Cell.Null) != Cell.Null
+
+    private fun plan(i: Int, op: ProviderOp, owners: Owners, memberships: Set<Long>): PlannedOp = when (op) {
        is ProviderOp.SetSyncState -> PlannedOp.SetSyncState(op.value)
        is ProviderOp.Insert -> {
            checkAccountColumns(i, op.table, op.values)
+            checkPhotoFile(i, op.table, op.values)
            checkReferences(i, op.table, op.values - op.refs.keys, owners)
            val values = if (op.table.scope == TableScope.ACCOUNT_COLUMNS) op.values + accountCells() else op.values
            PlannedOp.Insert(op.table, values, op.refs, op.yieldAllowed)
        }
        is ProviderOp.Update -> {
            checkAccountColumns(i, op.table, op.values)
+            checkPhotoFile(i, op.table, op.values)
            checkReferences(i, op.table, op.values, owners)
+            if (op.table == ProviderTable.DATA && writesUntypedData1(op.values)) {
+                checkMembershipUpdate(i, op, owners, memberships)
+            }
            checkTarget(i, op.table, op.target, owners)
            if (op.table == ProviderTable.EXTENDED_PROPERTIES) {
                // CalendarProvider updates extended properties only through
@@ -146,6 +170,35 @@ class ScopePlanner(private val account: AccountRef, private val lookup: ScopeLoo
        }
    }

+    /**
+     * `data1` written by an update that doesn't restate the mimetype: on a
+     * membership row it names a group. Rows chosen by a selection could be
+     * memberships the lookup didn't see, so only updates by id may do it.
+     */
+    private fun checkMembershipUpdate(i: Int, op: ProviderOp.Update, owners: Owners, memberships: Set<Long>) {
+        val id = op.target.id
+            ?: throw ProviderRefusal(BatchFailure.SCOPE, "op $i: data1 may only be written to a data row by id")
+        if (id !in memberships) return
+        val group = op.values[DATA1]?.rowId()
+            ?: throw ProviderRefusal(BatchFailure.PROVIDER, "op $i: a membership's data1 must be a group id")
+        when (owners.of(ProviderTable.GROUPS, group)) {
+            Owner.OURS -> Unit
+            Owner.FOREIGN -> throw ProviderRefusal(BatchFailure.SCOPE, "op $i: data.data1 names group $group of another account")
+            Owner.MISSING -> throw ProviderRefusal(BatchFailure.ASSERT, "op $i: data.data1 names group $group, which no longer exists")
+        }
+    }
+
+    /**
+     * A photo row's file id (`data14`) is the provider's: it points into the
+     * photo store, where another account's file id would let `readPhoto`
+     * read that account's photo. Photos are written as bytes (`data15`).
+     */
+    private fun checkPhotoFile(i: Int, table: ProviderTable, values: Map<String, Cell>) {
+        if (table == ProviderTable.DATA && (values[DATA14] ?: Cell.Null) != Cell.Null) {
+            throw ProviderRefusal(BatchFailure.SCOPE, "op $i: data.data14 (a photo file id) is written by the provider only")
+        }
+    }
+
    /** A row named by id must not belong to another account; a missing one fails on the expected count instead. */
    private fun checkTarget(i: Int, table: ProviderTable, target: RowTarget, owners: Owners) {
        val id = target.id ?: return
@@ -192,7 +245,7 @@ class ScopePlanner(private val account: AccountRef, private val lookup: ScopeLoo
    }

    /** Every row id the batch names, by table: op targets and referenced rows. */
-    private fun ownershipRequests(ops: List<ProviderOp>): Map<ProviderTable, Set<Long>> {
+    private fun ownershipRequests(ops: List<ProviderOp>, memberships: Set<Long>): Map<ProviderTable, Set<Long>> {
        val requests = HashMap<ProviderTable, MutableSet<Long>>()
        fun request(table: ProviderTable, id: Long) {
            requests.getOrPut(table) { HashSet() } += id
@@ -209,6 +262,7 @@ class ScopePlanner(private val account: AccountRef, private val lookup: ScopeLoo
                is ProviderOp.Update -> {
                    requestReferences(op.table, op.values)
                    op.target.id?.let { request(op.table, it) }
+                    if (op.target.id in memberships) op.values[DATA1]?.rowId()?.let { request(ProviderTable.GROUPS, it) }
                }
                is ProviderOp.Delete -> op.target.id?.let { request(op.table, it) }
                is ProviderOp.Assert -> op.target.id?.let { request(op.table, it) }
@@ -275,6 +329,9 @@ class ScopePlanner(private val account: AccountRef, private val lookup: ScopeLoo
        const val ACCOUNT_NAME = "account_name"
        const val ACCOUNT_TYPE = "account_type"
        const val DATA_SET = "data_set"
+        private const val MIMETYPE = "mimetype"
+        private const val DATA1 = "data1"
+        private const val DATA14 = "data14"

        /** Ids per `IN (…)` list: a few KB of selection, far from SQLite's and Binder's limits. */
        const val ID_LIST_CHUNK = 500
@@ -307,6 +364,9 @@ interface ScopeLookup {
    /** The raw contact of each data row that exists. */
    fun dataRawContacts(ids: Set<Long>): Map<Long, Long>

+    /** The mimetype of each data row that exists. */
+    fun dataMimetypes(ids: Set<Long>): Map<Long, String>
+
    /** The calendar of each event that exists. */
    fun eventCalendars(ids: Set<Long>): Map<Long, Long>