diff --git a/.idea/dictionaries/usernames.xml b/.idea/dictionaries/usernames.xml index 55dafdb672bf..24e5841bf6f9 100644 --- a/.idea/dictionaries/usernames.xml +++ b/.idea/dictionaries/usernames.xml @@ -83,6 +83,7 @@ quiro91 tarekkma vianey + voczi zanki diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/CrashReportService.kt b/AnkiDroid/src/main/java/com/ichi2/anki/CrashReportService.kt index 0cc97f963b53..ec1fda80df9b 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/CrashReportService.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/CrashReportService.kt @@ -18,6 +18,8 @@ package com.ichi2.anki import android.app.Application import android.content.Context import android.content.SharedPreferences +import android.util.Patterns +import androidx.annotation.CheckResult import androidx.annotation.StringRes import androidx.annotation.VisibleForTesting import androidx.core.content.edit @@ -28,9 +30,11 @@ import com.ichi2.anki.analytics.UsageAnalytics import com.ichi2.anki.analytics.UsageAnalytics.sendAnalyticsException import com.ichi2.anki.exception.ManuallyReportedException import com.ichi2.anki.exception.UserSubmittedException +import com.ichi2.anki.preferences.get import com.ichi2.anki.preferences.sharedPrefs import com.ichi2.libanki.utils.TimeManager import com.ichi2.utils.WebViewDebugging.setDataDirectorySuffix +import net.ankiweb.rsdroid.exceptions.BackendSyncException.BackendSyncServerMessageException import org.acra.ACRA import org.acra.ReportField import org.acra.config.CoreConfigurationBuilder @@ -281,12 +285,50 @@ object CrashReportService { } } if (FEEDBACK_REPORT_NEVER != reportMode) { + if (e.removePII() == null) { + return + } ACRA.errorReporter.putCustomData("origin", origin ?: "") ACRA.errorReporter.putCustomData("additionalInfo", additionalInfo ?: "") ACRA.errorReporter.handleException(e) } } + private fun Throwable.replaceMessage(newMessage: String?): Throwable = + Exception(newMessage, this.cause) + + private val emailRegex = Patterns.EMAIL_ADDRESS.toRegex() + private val throwableRules: List<(Throwable) -> Throwable?> = listOf( + { e -> + // BackendSyncServerMessage may contain PII and we do not want this leaked to ACRA. + // If the exception is of a cause deemed to be associated with a message containing PII, + // then do not send it at all to retain user privacy. + // Related: https://github.com/ankidroid/Anki-Android/issues/17392 + // and also https://github.com/ankitects/anki/commit/ba1f5f4 + if (e.cause is BackendSyncServerMessageException) null else e + }, + { e -> + // Make sure we filter out any email addresses which may be leaked by the sync server. + e.replaceMessage(emailRegex.replace(e.message.orEmpty(), "-EMAIL-")) + }, + { e -> + // Filter out the current username in the message (if it exists) as it is considered PII. + e.replaceMessage( + e.message.orEmpty().replace( + mApplication.sharedPrefs().get(SyncPreferences.USERNAME).toString(), + "-USERNAME-" + ) + ) + } + ) + + // Strip Personally Identifiable Information (PII) from Exception.message + // or return null if we should discard completely. + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + @CheckResult + fun Throwable.removePII(): Throwable? = + throwableRules.fold(this) { nextThrowable, rule -> rule.invoke(nextThrowable) ?: return null } + fun isProperServiceProcess(): Boolean { return ACRA.isACRASenderServiceProcess() } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/CrashReportServiceTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/CrashReportServiceTest.kt new file mode 100644 index 000000000000..8967ea912cf2 --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/CrashReportServiceTest.kt @@ -0,0 +1,65 @@ +/* + * Copyright (c) 2024 voczi + * + * This program is free software; you can redistribute it and/or modify it under + * the terms of the GNU General Public License as published by the Free Software + * Foundation; either version 3 of the License, or (at your option) any later + * version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT ANY + * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A + * PARTICULAR PURPOSE. See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see . + */ + +package com.ichi2.anki + +import androidx.core.content.edit +import androidx.test.ext.junit.runners.AndroidJUnit4 +import anki.backend.BackendError +import com.ichi2.anki.CrashReportService.removePII +import com.ichi2.anki.preferences.sharedPrefs +import net.ankiweb.rsdroid.exceptions.BackendDeckIsFilteredException +import net.ankiweb.rsdroid.exceptions.BackendSyncException.BackendSyncServerMessageException +import org.hamcrest.MatcherAssert.assertThat +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mockito.mock +import kotlin.Throwable +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull + +@RunWith(AndroidJUnit4::class) +class CrashReportServiceTest : RobolectricTest() { + @Test + fun `Normal exceptions are untouched`() { + val message = "This is a test message which should not be filtered nor blocked" + val processed = BackendDeckIsFilteredException(BackendError.newBuilder().setMessage(message).build()).removePII() + + assertNotNull(processed, "Returned exception should not be null") + assertEquals(processed.message!!, message, "Returned exception's message should stay the same") + } + + @Test + fun `Backend sync server messages are blocked`() { + assertNull(mock(BackendSyncServerMessageException::class.java).removePII(), "Returned exception is null") + } + + @Test + fun `Emails are filtered out`() { + val msg = Throwable("Lorem ipsum 123 email@domain.tld Lorem ipsum.").removePII()!!.message!! + assertThat("Exception message does not contain the email", !msg.contains("email@domain.tld")) + } + + @Test + fun `Username is filtered out`() { + val preferences = targetContext.sharedPrefs() + preferences.edit() { putString(SyncPreferences.USERNAME, "MyUser") } + + val msg = Throwable("Lorem ipsum 123 MyUser Lorem ipsum.").removePII()!!.message!! + assertThat("Exception message not contain the username", !msg.contains("MyUser")) + } +}