diff --git a/android/app/src/main/java/com/anonymous/bulwarkmobile/sync/ProviderIo.kt b/android/app/src/main/java/com/anonymous/bulwarkmobile/sync/ProviderIo.kt index cb0e250d..37f23290 100644 --- a/android/app/src/main/java/com/anonymous/bulwarkmobile/sync/ProviderIo.kt +++ b/android/app/src/main/java/com/anonymous/bulwarkmobile/sync/ProviderIo.kt @@ -199,6 +199,14 @@ class ProviderIo(context: Context, accountName: String, private val authority: S override fun dataRawContacts(ids: Set): Map = idPairs(ProviderTable.DATA, "raw_contact_id", ids) + override fun dataMimetypes(ids: Set): Map { + val mimetypes = HashMap() + 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): Map = idPairs(ProviderTable.EVENTS, "calendar_id", ids) diff --git a/android/app/src/main/java/com/anonymous/bulwarkmobile/sync/ProviderOps.kt b/android/app/src/main/java/com/anonymous/bulwarkmobile/sync/ProviderOps.kt index 167bbd4d..b5d9555b 100644 --- a/android/app/src/main/java/com/anonymous/bulwarkmobile/sync/ProviderOps.kt +++ b/android/app/src/main/java/com/anonymous/bulwarkmobile/sync/ProviderOps.kt @@ -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 { @@ -323,7 +379,7 @@ object ProviderOpParser { } private fun JSONObject.whereArgs(where: String): Pair> { - 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 -> diff --git a/android/app/src/main/java/com/anonymous/bulwarkmobile/sync/ProviderScope.kt b/android/app/src/main/java/com/anonymous/bulwarkmobile/sync/ProviderScope.kt index 01729241..7706b0a1 100644 --- a/android/app/src/main/java/com/anonymous/bulwarkmobile/sync/ProviderScope.kt +++ b/android/app/src/main/java/com/anonymous/bulwarkmobile/sync/ProviderScope.kt @@ -45,21 +45,45 @@ class ScopePlanner(private val account: AccountRef, private val lookup: ScopeLoo } fun planBatch(ops: List): List { - 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): Set { + val ids = ops.asSequence() + .filterIsInstance() + .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) = + MIMETYPE !in values && (values[DATA1] ?: Cell.Null) != Cell.Null + + private fun plan(i: Int, op: ProviderOp, owners: Owners, memberships: Set): 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) { + 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) { + 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): Map> { + private fun ownershipRequests(ops: List, memberships: Set): Map> { val requests = HashMap>() 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): Map + /** The mimetype of each data row that exists. */ + fun dataMimetypes(ids: Set): Map + /** The calendar of each event that exists. */ fun eventCalendars(ids: Set): Map diff --git a/android/app/src/test/java/com/anonymous/bulwarkmobile/sync/ProviderOpsTest.kt b/android/app/src/test/java/com/anonymous/bulwarkmobile/sync/ProviderOpsTest.kt index 751f0ad5..2d1f6930 100644 --- a/android/app/src/test/java/com/anonymous/bulwarkmobile/sync/ProviderOpsTest.kt +++ b/android/app/src/test/java/com/anonymous/bulwarkmobile/sync/ProviderOpsTest.kt @@ -23,6 +23,8 @@ private class FakeLookup(private val me: AccountRef = ME) : ScopeLookup { val colors = HashMap() /** data id → raw contact */ val data = HashMap() + /** data id → mimetype */ + val mimetypes = HashMap() /** event id → calendar */ val events = HashMap() /** child table → child id → event */ @@ -42,6 +44,8 @@ private class FakeLookup(private val me: AccountRef = ME) : ScopeLookup { override fun dataRawContacts(ids: Set) = data.filterKeys { it in ids } + override fun dataMimetypes(ids: Set) = mimetypes.filterKeys { it in ids } + override fun eventCalendars(ids: Set) = events.filterKeys { it in ids } override fun childEvents(table: ProviderTable, ids: Set): Map = @@ -306,6 +310,49 @@ class ProviderOpsTest { plan(CONTACTS, """{"op":"insert","table":"data","values":{"raw_contact_id":1,"mimetype":"x","data1":11}}""") } + @Test + fun `a membership row's group is checked on updates by id too`() { + lookup.mimetypes[100] = GROUP_MEMBERSHIP_MIMETYPE + plan(CONTACTS, """{"op":"update","table":"data","id":100,"values":{"data1":10}}""") + refused(BatchFailure.SCOPE) { plan(CONTACTS, """{"op":"update","table":"data","id":100,"values":{"data1":11}}""") } + refused(BatchFailure.ASSERT) { plan(CONTACTS, """{"op":"update","table":"data","id":100,"values":{"data1":12}}""") } + // data1 by a selection could reach memberships the lookup never saw. + refused(BatchFailure.SCOPE) { + plan(CONTACTS, """{"op":"update","table":"data","where":"raw_contact_id = ?","args":[1],"values":{"data1":11}}""") + } + // data1 of other kinds is plain data. + lookup.mimetypes[100] = "vnd.android.cursor.item/phone_v2" + plan(CONTACTS, """{"op":"update","table":"data","id":100,"values":{"data1":11}}""") + } + + @Test + fun `photo file ids are never written`() { + val photo = """"raw_contact_id":1,"mimetype":"vnd.android.cursor.item/photo"""" + plan(CONTACTS, """{"op":"insert","table":"data","values":{$photo,"data14":null,"data15":{"b64":"AAAA"}}}""") + refused(BatchFailure.SCOPE) { plan(CONTACTS, """{"op":"insert","table":"data","values":{$photo,"data14":5}}""") } + refused(BatchFailure.SCOPE) { plan(CONTACTS, """{"op":"update","table":"data","id":100,"values":{"data14":5}}""") } + } + + @Test + fun `a selection can't close the scope's parentheses or query elsewhere`() { + for (where in listOf("1) OR (1", "dirty = 1) OR (1 = 1", "(1", "1; DELETE FROM events", "1 -- x", "1 /* x */", + "_id IN (SELECT _id FROM events)", "1 UNION ALL x", "title = 'open")) { + val quoted = JSONObject.quote(where) + refused(BatchFailure.SCOPE) { plan(CALENDAR, """{"op":"update","table":"events","where":$quoted,"values":{"dirty":1}}""") } + refused(BatchFailure.SCOPE) { plan(CALENDAR, """{"op":"delete","table":"attendees","where":$quoted}""") } + refused(BatchFailure.SCOPE) { + ProviderOpParser.parseQuery(CALENDAR, """{"table":"attendees","columns":["_id"],"where":$quoted}""") + } + } + refused(BatchFailure.SCOPE) { + ProviderOpParser.parseQuery(CONTACTS, """{"table":"raw_contacts","columns":["_id"],"orderBy":"_id); DROP TABLE x; --"}""") + } + // Parentheses and keywords inside quoted literals are data. + plan(CALENDAR, """{"op":"update","table":"events","where":"title = '(x' OR title = 'select'","values":{"dirty":1}}""") + plan(CALENDAR, """{"op":"assert","table":"events","where":"(dirty = 1 OR deleted = 1) AND _sync_id LIKE '~pending/%'","expectCount":0}""") + ProviderOpParser.parseQuery(CONTACTS, """{"table":"raw_contacts","columns":["_id"],"orderBy":"_id DESC"}""") + } + @Test fun `moving a row to another parent is checked like an insert`() { refused(BatchFailure.SCOPE) { plan(CALENDAR, """{"op":"update","table":"events","id":30,"values":{"calendar_id":22}}""") }