diff --git a/app/src/main/java/org/schabi/newpipe/settings/export/ImportExportManager.kt b/app/src/main/java/org/schabi/newpipe/settings/export/ImportExportManager.kt index 339ebf644..5558a1b37 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/export/ImportExportManager.kt +++ b/app/src/main/java/org/schabi/newpipe/settings/export/ImportExportManager.kt @@ -9,6 +9,7 @@ import com.grack.nanojson.JsonWriter import org.schabi.newpipe.streams.io.SharpOutputStream import org.schabi.newpipe.streams.io.StoredFileHelper import org.schabi.newpipe.util.ZipHelper +import java.io.FileNotFoundException import java.io.IOException import java.io.ObjectOutputStream import java.util.zip.ZipOutputStream @@ -110,10 +111,12 @@ class ImportExportManager(private val fileLocator: BackupFileLocator) { fun loadSerializedPrefs(zipFile: StoredFileHelper, preferences: SharedPreferences) { ZipHelper.extractFileFromZip(zipFile, BackupFileLocator.FILE_NAME_SERIALIZED_PREFS) { PreferencesObjectInputStream(it).use { input -> - val editor = preferences.edit() - editor.clear() @Suppress("UNCHECKED_CAST") val entries = input.readObject() as Map + + val editor = preferences.edit() + editor.clear() + for ((key, value) in entries) { when (value) { is Boolean -> editor.putBoolean(key, value) @@ -133,19 +136,24 @@ class ImportExportManager(private val fileLocator: BackupFileLocator) { Log.e(TAG, "Unable to loadSerializedPrefs") } } + }.let { fileExists -> + if (!fileExists) { + throw FileNotFoundException(BackupFileLocator.FILE_NAME_SERIALIZED_PREFS) + } } } /** * Remove all shared preferences from the app and load the preferences supplied to the manager. */ - @Throws(JsonParserException::class) + @Throws(IOException::class, JsonParserException::class) fun loadJsonPrefs(zipFile: StoredFileHelper, preferences: SharedPreferences) { ZipHelper.extractFileFromZip(zipFile, BackupFileLocator.FILE_NAME_JSON_PREFS) { + val jsonObject = JsonParser.`object`().from(it) + val editor = preferences.edit() editor.clear() - val jsonObject = JsonParser.`object`().from(it) for ((key, value) in jsonObject) { when (value) { is Boolean -> editor.putBoolean(key, value) @@ -162,6 +170,10 @@ class ImportExportManager(private val fileLocator: BackupFileLocator) { if (!editor.commit()) { Log.e(TAG, "Unable to loadJsonPrefs") } + }.let { fileExists -> + if (!fileExists) { + throw FileNotFoundException(BackupFileLocator.FILE_NAME_JSON_PREFS) + } } } } diff --git a/app/src/test/java/org/schabi/newpipe/settings/ImportAllCombinationsTest.kt b/app/src/test/java/org/schabi/newpipe/settings/ImportAllCombinationsTest.kt new file mode 100644 index 000000000..862ac3b80 --- /dev/null +++ b/app/src/test/java/org/schabi/newpipe/settings/ImportAllCombinationsTest.kt @@ -0,0 +1,184 @@ +package org.schabi.newpipe.settings + +import android.content.SharedPreferences +import org.junit.Assert +import org.junit.Test +import org.mockito.Mockito +import org.schabi.newpipe.settings.export.BackupFileLocator +import org.schabi.newpipe.settings.export.ImportExportManager +import org.schabi.newpipe.streams.io.StoredFileHelper +import us.shandian.giga.io.FileStream +import java.io.File +import java.io.IOException +import java.nio.file.Files + +class ImportAllCombinationsTest { + + companion object { + private val classloader = ImportExportManager::class.java.classLoader!! + } + + private enum class Ser(val id: String) { + YES("ser"), + VULNERABLE("vulnser"), + NO("noser"); + } + + private data class FailData( + val containsDb: Boolean, + val containsSer: Ser, + val containsJson: Boolean, + val filename: String, + val throwable: Throwable, + ) + + private fun testZipCombination( + containsDb: Boolean, + containsSer: Ser, + containsJson: Boolean, + filename: String, + runTest: (test: () -> Unit) -> Unit, + ) { + val zipFile = File(classloader.getResource(filename)?.file!!) + val zip = Mockito.mock(StoredFileHelper::class.java, Mockito.withSettings().stubOnly()) + Mockito.`when`(zip.stream).then { FileStream(zipFile) } + + val fileLocator = Mockito.mock( + BackupFileLocator::class.java, + Mockito.withSettings().stubOnly() + ) + val db = File.createTempFile("newpipe_", "") + val dbJournal = File.createTempFile("newpipe_", "") + val dbWal = File.createTempFile("newpipe_", "") + val dbShm = File.createTempFile("newpipe_", "") + Mockito.`when`(fileLocator.db).thenReturn(db) + Mockito.`when`(fileLocator.dbJournal).thenReturn(dbJournal) + Mockito.`when`(fileLocator.dbShm).thenReturn(dbShm) + Mockito.`when`(fileLocator.dbWal).thenReturn(dbWal) + + if (containsDb) { + runTest { + Assert.assertTrue(ImportExportManager(fileLocator).extractDb(zip)) + Assert.assertFalse(dbJournal.exists()) + Assert.assertFalse(dbWal.exists()) + Assert.assertFalse(dbShm.exists()) + Assert.assertTrue("database file size is zero", Files.size(db.toPath()) > 0) + } + } else { + runTest { + Assert.assertFalse(ImportExportManager(fileLocator).extractDb(zip)) + Assert.assertTrue(dbJournal.exists()) + Assert.assertTrue(dbWal.exists()) + Assert.assertTrue(dbShm.exists()) + Assert.assertEquals(0, Files.size(db.toPath())) + } + } + + val preferences = Mockito.mock(SharedPreferences::class.java, Mockito.withSettings().stubOnly()) + var editor = Mockito.mock(SharedPreferences.Editor::class.java) + Mockito.`when`(preferences.edit()).thenReturn(editor) + Mockito.`when`(editor.commit()).thenReturn(true) + + when (containsSer) { + Ser.YES -> runTest { + Assert.assertTrue(ImportExportManager(fileLocator).exportHasSerializedPrefs(zip)) + ImportExportManager(fileLocator).loadSerializedPrefs(zip, preferences) + + Mockito.verify(editor, Mockito.times(1)).clear() + Mockito.verify(editor, Mockito.times(1)).commit() + Mockito.verify(editor, Mockito.atLeastOnce()) + .putBoolean(Mockito.anyString(), Mockito.anyBoolean()) + Mockito.verify(editor, Mockito.atLeastOnce()) + .putString(Mockito.anyString(), Mockito.anyString()) + Mockito.verify(editor, Mockito.atLeastOnce()) + .putInt(Mockito.anyString(), Mockito.anyInt()) + } + Ser.VULNERABLE -> runTest { + Assert.assertTrue(ImportExportManager(fileLocator).exportHasSerializedPrefs(zip)) + Assert.assertThrows(ClassNotFoundException::class.java) { + ImportExportManager(fileLocator).loadSerializedPrefs(zip, preferences) + } + + Mockito.verify(editor, Mockito.never()).clear() + Mockito.verify(editor, Mockito.never()).commit() + } + Ser.NO -> runTest { + Assert.assertFalse(ImportExportManager(fileLocator).exportHasSerializedPrefs(zip)) + Assert.assertThrows(IOException::class.java) { + ImportExportManager(fileLocator).loadSerializedPrefs(zip, preferences) + } + + Mockito.verify(editor, Mockito.never()).clear() + Mockito.verify(editor, Mockito.never()).commit() + } + } + + // recreate editor mock so verify() behaves correctly + editor = Mockito.mock(SharedPreferences.Editor::class.java) + Mockito.`when`(preferences.edit()).thenReturn(editor) + Mockito.`when`(editor.commit()).thenReturn(true) + + if (containsJson) { + runTest { + Assert.assertTrue(ImportExportManager(fileLocator).exportHasJsonPrefs(zip)) + ImportExportManager(fileLocator).loadJsonPrefs(zip, preferences) + + Mockito.verify(editor, Mockito.times(1)).clear() + Mockito.verify(editor, Mockito.times(1)).commit() + Mockito.verify(editor, Mockito.atLeastOnce()) + .putBoolean(Mockito.anyString(), Mockito.anyBoolean()) + Mockito.verify(editor, Mockito.atLeastOnce()) + .putString(Mockito.anyString(), Mockito.anyString()) + Mockito.verify(editor, Mockito.atLeastOnce()) + .putInt(Mockito.anyString(), Mockito.anyInt()) + } + } else { + runTest { + Assert.assertFalse(ImportExportManager(fileLocator).exportHasJsonPrefs(zip)) + Assert.assertThrows(IOException::class.java) { + ImportExportManager(fileLocator).loadJsonPrefs(zip, preferences) + } + + Mockito.verify(editor, Mockito.never()).clear() + Mockito.verify(editor, Mockito.never()).commit() + } + } + } + + @Test + fun `Importing all possible combinations of zip files`() { + val failedAssertions = mutableListOf() + for (containsDb in listOf(true, false)) { + for (containsSer in Ser.entries) { + for (containsJson in listOf(true, false)) { + val filename = "settings/${if (containsDb) "db" else "nodb"}_${ + containsSer.id}_${if (containsJson) "json" else "nojson"}.zip" + testZipCombination(containsDb, containsSer, containsJson, filename) { test -> + try { + test() + } catch (e: Throwable) { + failedAssertions.add( + FailData( + containsDb, containsSer, containsJson, + filename, e + ) + ) + } + } + } + } + } + + if (failedAssertions.isNotEmpty()) { + for (a in failedAssertions) { + println( + "Assertion failed with containsDb=${a.containsDb}, containsSer=${ + a.containsSer}, containsJson=${a.containsJson}, filename=${a.filename}:" + ) + a.throwable.printStackTrace() + println() + } + Assert.fail("${failedAssertions.size} assertions failed") + } + } +} diff --git a/app/src/test/java/org/schabi/newpipe/settings/ImportExportManagerTest.kt b/app/src/test/java/org/schabi/newpipe/settings/ImportExportManagerTest.kt index 70420801c..a524f64f3 100644 --- a/app/src/test/java/org/schabi/newpipe/settings/ImportExportManagerTest.kt +++ b/app/src/test/java/org/schabi/newpipe/settings/ImportExportManagerTest.kt @@ -109,7 +109,7 @@ class ImportExportManagerTest { `when`(fileLocator.dbShm).thenReturn(dbShm) `when`(fileLocator.dbWal).thenReturn(dbWal) - val zip = File(classloader.getResource("settings/newpipe.zip")?.file!!) + val zip = File(classloader.getResource("settings/db_ser_json.zip")?.file!!) `when`(storedFileHelper.stream).thenReturn(FileStream(zip)) val success = ImportExportManager(fileLocator).extractDb(storedFileHelper) @@ -128,7 +128,7 @@ class ImportExportManagerTest { val dbShm = File.createTempFile("newpipe_", "") `when`(fileLocator.db).thenReturn(db) - val emptyZip = File(classloader.getResource("settings/empty.zip")?.file!!) + val emptyZip = File(classloader.getResource("settings/nodb_noser_nojson.zip")?.file!!) `when`(storedFileHelper.stream).thenReturn(FileStream(emptyZip)) val success = ImportExportManager(fileLocator).extractDb(storedFileHelper) @@ -141,21 +141,21 @@ class ImportExportManagerTest { @Test fun `Contains setting must return true if a settings file exists in the zip`() { - val zip = File(classloader.getResource("settings/newpipe.zip")?.file!!) + val zip = File(classloader.getResource("settings/db_ser_json.zip")?.file!!) `when`(storedFileHelper.stream).thenReturn(FileStream(zip)) assertTrue(ImportExportManager(fileLocator).exportHasSerializedPrefs(storedFileHelper)) } @Test - fun `Contains setting must return false if a no settings file exists in the zip`() { - val emptyZip = File(classloader.getResource("settings/empty.zip")?.file!!) + fun `Contains setting must return false if no settings file exists in the zip`() { + val emptyZip = File(classloader.getResource("settings/nodb_noser_nojson.zip")?.file!!) `when`(storedFileHelper.stream).thenReturn(FileStream(emptyZip)) assertFalse(ImportExportManager(fileLocator).exportHasSerializedPrefs(storedFileHelper)) } @Test fun `Preferences must be set from the settings file`() { - val zip = File(classloader.getResource("settings/newpipe.zip")?.file!!) + val zip = File(classloader.getResource("settings/db_ser_json.zip")?.file!!) `when`(storedFileHelper.stream).thenReturn(FileStream(zip)) val preferences = Mockito.mock(SharedPreferences::class.java, withSettings().stubOnly()) @@ -172,12 +172,10 @@ class ImportExportManagerTest { @Test fun `Importing preferences with a serialization injected class should fail`() { - val emptyZip = File(classloader.getResource("settings/vulnerable_serialization.zip")?.file!!) + val emptyZip = File(classloader.getResource("settings/db_vulnser_json.zip")?.file!!) `when`(storedFileHelper.stream).thenReturn(FileStream(emptyZip)) val preferences = Mockito.mock(SharedPreferences::class.java, withSettings().stubOnly()) - val editor = Mockito.mock(SharedPreferences.Editor::class.java) - `when`(preferences.edit()).thenReturn(editor) assertThrows(ClassNotFoundException::class.java) { ImportExportManager(fileLocator).loadSerializedPrefs(storedFileHelper, preferences) diff --git a/app/src/test/resources/settings/README.md b/app/src/test/resources/settings/README.md new file mode 100644 index 000000000..13a3440e1 --- /dev/null +++ b/app/src/test/resources/settings/README.md @@ -0,0 +1,4 @@ +`*.zip` files in this folder are NewPipe database exports, in all possible configurations: +- `db` / `nodb` indicates if there is a `newpipe.db` database included or not +- `ser` / `vulnser` / `noser` indicates if there is a `newpipe.settings` Java-serialized preferences file included, if it is included and contains an injection attack, of if it is not included +- `json` / `nojson` indicates if there is a `preferences.json` JSON preferences file included or not diff --git a/app/src/test/resources/settings/db_noser_json.zip b/app/src/test/resources/settings/db_noser_json.zip new file mode 100644 index 000000000..4bbb7523c Binary files /dev/null and b/app/src/test/resources/settings/db_noser_json.zip differ diff --git a/app/src/test/resources/settings/db_noser_nojson.zip b/app/src/test/resources/settings/db_noser_nojson.zip new file mode 100644 index 000000000..625947ef5 Binary files /dev/null and b/app/src/test/resources/settings/db_noser_nojson.zip differ diff --git a/app/src/test/resources/settings/db_ser_json.zip b/app/src/test/resources/settings/db_ser_json.zip new file mode 100644 index 000000000..3662666cb Binary files /dev/null and b/app/src/test/resources/settings/db_ser_json.zip differ diff --git a/app/src/test/resources/settings/db_ser_nojson.zip b/app/src/test/resources/settings/db_ser_nojson.zip new file mode 100644 index 000000000..0196c9f64 Binary files /dev/null and b/app/src/test/resources/settings/db_ser_nojson.zip differ diff --git a/app/src/test/resources/settings/db_vulnser_json.zip b/app/src/test/resources/settings/db_vulnser_json.zip new file mode 100644 index 000000000..64444acdd Binary files /dev/null and b/app/src/test/resources/settings/db_vulnser_json.zip differ diff --git a/app/src/test/resources/settings/db_vulnser_nojson.zip b/app/src/test/resources/settings/db_vulnser_nojson.zip new file mode 100644 index 000000000..56d58a22e Binary files /dev/null and b/app/src/test/resources/settings/db_vulnser_nojson.zip differ diff --git a/app/src/test/resources/settings/newpipe.zip b/app/src/test/resources/settings/newpipe.zip deleted file mode 100644 index 1ce8431fe..000000000 Binary files a/app/src/test/resources/settings/newpipe.zip and /dev/null differ diff --git a/app/src/test/resources/settings/nodb_noser_json.zip b/app/src/test/resources/settings/nodb_noser_json.zip new file mode 100644 index 000000000..7881f939b Binary files /dev/null and b/app/src/test/resources/settings/nodb_noser_json.zip differ diff --git a/app/src/test/resources/settings/empty.zip b/app/src/test/resources/settings/nodb_noser_nojson.zip similarity index 100% rename from app/src/test/resources/settings/empty.zip rename to app/src/test/resources/settings/nodb_noser_nojson.zip diff --git a/app/src/test/resources/settings/nodb_ser_json.zip b/app/src/test/resources/settings/nodb_ser_json.zip new file mode 100644 index 000000000..aa8316c6c Binary files /dev/null and b/app/src/test/resources/settings/nodb_ser_json.zip differ diff --git a/app/src/test/resources/settings/nodb_ser_nojson.zip b/app/src/test/resources/settings/nodb_ser_nojson.zip new file mode 100644 index 000000000..437d30d8b Binary files /dev/null and b/app/src/test/resources/settings/nodb_ser_nojson.zip differ diff --git a/app/src/test/resources/settings/nodb_vulnser_json.zip b/app/src/test/resources/settings/nodb_vulnser_json.zip new file mode 100644 index 000000000..84f97aac0 Binary files /dev/null and b/app/src/test/resources/settings/nodb_vulnser_json.zip differ diff --git a/app/src/test/resources/settings/nodb_vulnser_nojson.zip b/app/src/test/resources/settings/nodb_vulnser_nojson.zip new file mode 100644 index 000000000..26a2c3b90 Binary files /dev/null and b/app/src/test/resources/settings/nodb_vulnser_nojson.zip differ diff --git a/app/src/test/resources/settings/vulnerable_serialization.zip b/app/src/test/resources/settings/vulnerable_serialization.zip deleted file mode 100644 index d57a5f8d0..000000000 Binary files a/app/src/test/resources/settings/vulnerable_serialization.zip and /dev/null differ