From 5470469d73eefec4cfe87c5642f45ef58ccf49c3 Mon Sep 17 00:00:00 2001 From: Suprabh Shukla Date: Fri, 12 May 2017 15:26:54 -0700 Subject: [PATCH] Clearing up invalid entries when SyncStorageEngine starts Any app with permission WRITE_SYNC_SETTINGS could write sync settings for authorities or accounts that are not valid. This results in invalid data being persisted to disk which can effectively lead to a DOS style attack. Clearing such entries on boot will make sure that a reboot fixes any such issues. Test: cts-tradefed run cts-dev -m CtsSyncContentHostTestCases Bug: 35028827 Change-Id: I9e206a42508e3cba65d7523bf47fff743f47dcb2 Merged-In: I9e206a42508e3cba65d7523bf47fff743f47dcb2 (cherry picked from commit 042a478b73c3b7f7cd73f5bb1af657cfe07d0571) --- .../android/server/content/SyncStorageEngine.java | 65 ++++++++++++++++++++-- 1 file changed, 60 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/content/SyncStorageEngine.java b/services/core/java/com/android/server/content/SyncStorageEngine.java index 96a7bb47b5e2..ad4a819fc551 100644 --- a/services/core/java/com/android/server/content/SyncStorageEngine.java +++ b/services/core/java/com/android/server/content/SyncStorageEngine.java @@ -18,6 +18,7 @@ package com.android.server.content; import android.accounts.Account; import android.accounts.AccountAndUser; +import android.accounts.AccountManager; import android.app.backup.BackupManager; import android.content.ComponentName; import android.content.ContentResolver; @@ -27,6 +28,7 @@ import android.content.PeriodicSync; import android.content.SyncInfo; import android.content.SyncRequest; import android.content.SyncStatusInfo; +import android.content.pm.PackageManager; import android.database.Cursor; import android.database.sqlite.SQLiteDatabase; import android.database.sqlite.SQLiteException; @@ -429,6 +431,49 @@ public class SyncStorageEngine extends Handler { public void onSyncRequest(EndPoint info, int reason, Bundle extras); } + /** + * Validator that maintains a lazy cache of accounts and providers to tell if an authority or + * account is valid. + */ + private static class AccountAuthorityValidator { + final private AccountManager mAccountManager; + final private PackageManager mPackageManager; + final private SparseArray mAccountsCache; + final private SparseArray> mProvidersPerUserCache; + + AccountAuthorityValidator(Context context) { + mAccountManager = context.getSystemService(AccountManager.class); + mPackageManager = context.getPackageManager(); + mAccountsCache = new SparseArray<>(); + mProvidersPerUserCache = new SparseArray<>(); + } + + // An account is valid if an installed authenticator has previously created that account + // on the device + boolean isAccountValid(Account account, int userId) { + Account[] accountsForUser = mAccountsCache.get(userId); + if (accountsForUser == null) { + accountsForUser = mAccountManager.getAccountsAsUser(userId); + mAccountsCache.put(userId, accountsForUser); + } + return ArrayUtils.contains(accountsForUser, account); + } + + // An authority is only valid if it has a content provider installed on the system + boolean isAuthorityValid(String authority, int userId) { + ArrayMap authorityMap = mProvidersPerUserCache.get(userId); + if (authorityMap == null) { + authorityMap = new ArrayMap<>(); + mProvidersPerUserCache.put(userId, authorityMap); + } + if (!authorityMap.containsKey(authority)) { + authorityMap.put(authority, + mPackageManager.resolveContentProviderAsUser(authority, 0, userId) != null); + } + return authorityMap.get(authority); + } + } + // Primary list of all syncable authorities. Also our global lock. private final SparseArray mAuthorities = new SparseArray(); @@ -1894,12 +1939,13 @@ public class SyncStorageEngine extends Handler { eventType = parser.next(); AuthorityInfo authority = null; PeriodicSync periodicSync = null; + AccountAuthorityValidator validator = new AccountAuthorityValidator(mContext); do { if (eventType == XmlPullParser.START_TAG) { tagName = parser.getName(); if (parser.getDepth() == 2) { if ("authority".equals(tagName)) { - authority = parseAuthority(parser, version); + authority = parseAuthority(parser, version, validator); periodicSync = null; if (authority != null) { if (authority.ident > highestAuthorityId) { @@ -2032,7 +2078,8 @@ public class SyncStorageEngine extends Handler { mMasterSyncAutomatically.put(userId, listen); } - private AuthorityInfo parseAuthority(XmlPullParser parser, int version) { + private AuthorityInfo parseAuthority(XmlPullParser parser, int version, + AccountAuthorityValidator validator) { AuthorityInfo authority = null; int id = -1; try { @@ -2077,12 +2124,22 @@ public class SyncStorageEngine extends Handler { info = new EndPoint( new Account(accountName, accountType), authorityName, userId); + if (validator.isAccountValid(info.account, userId) + && validator.isAuthorityValid(authorityName, userId)) { + authority = getOrCreateAuthorityLocked(info, id, false); + } else { + EventLog.writeEvent(0x534e4554, "35028827", -1, + "account:" + info.account + " provider:" + authorityName + " user:" + + userId); + } } else { info = new EndPoint( new ComponentName(packageName, className), userId); + authority = getOrCreateAuthorityLocked(info, id, false); } - authority = getOrCreateAuthorityLocked(info, id, false); + } + if (authority != null) { // If the version is 0 then we are upgrading from a file format that did not // know about periodic syncs. In that case don't clear the list since we // want the default, which is a daily periodic sync. @@ -2091,8 +2148,6 @@ public class SyncStorageEngine extends Handler { if (version > 0) { authority.periodicSyncs.clear(); } - } - if (authority != null) { authority.enabled = enabled == null || Boolean.parseBoolean(enabled); try { authority.syncable = (syncable == null) ? -- 2.11.0