fix(format/common): validate TOTP secret ahead of time

Fixes #2949
This commit is contained in:
Harsh Shandilya 2024-03-26 01:41:31 +05:30
parent 825c8af377
commit 17d4b803f7
2 changed files with 34 additions and 13 deletions

View file

@ -9,6 +9,9 @@ import androidx.annotation.VisibleForTesting
import app.passwordstore.util.time.UserClock import app.passwordstore.util.time.UserClock
import app.passwordstore.util.totp.Otp import app.passwordstore.util.totp.Otp
import app.passwordstore.util.totp.TotpFinder import app.passwordstore.util.totp.TotpFinder
import com.github.michaelbull.result.Err
import com.github.michaelbull.result.Ok
import com.github.michaelbull.result.Result
import com.github.michaelbull.result.mapBoth import com.github.michaelbull.result.mapBoth
import dagger.assisted.Assisted import dagger.assisted.Assisted
import dagger.assisted.AssistedFactory import dagger.assisted.AssistedFactory
@ -60,14 +63,22 @@ constructor(
require(totpSecret != null) { "Cannot collect this flow without a TOTP secret" } require(totpSecret != null) { "Cannot collect this flow without a TOTP secret" }
do { do {
val otp = calculateTotp() val otp = calculateTotp()
emit(otp) if (otp.isOk) {
delay(THOUSAND_MILLIS.milliseconds) emit(otp.value)
delay(THOUSAND_MILLIS.milliseconds)
} else {
throw otp.error
}
} while (coroutineContext.isActive) } while (coroutineContext.isActive)
} }
/** Obtain the [Totp.value] for this [PasswordEntry] at the current time. */ /** Obtain the [Totp.value] for this [PasswordEntry] at the current time. */
public val currentOtp: String public val currentOtp: String
get() = calculateTotp().value get() {
val otp = calculateTotp()
check(otp.isOk)
return otp.value.value
}
/** /**
* String representation of [extraContent] but with authentication related data such as TOTP URIs * String representation of [extraContent] but with authentication related data such as TOTP URIs
@ -83,7 +94,14 @@ constructor(
extraContentWithoutAuthData = generateExtraContentWithoutAuthData() extraContentWithoutAuthData = generateExtraContentWithoutAuthData()
extraContent = generateExtraContentPairs() extraContent = generateExtraContentPairs()
username = findUsername() username = findUsername()
totpSecret = totpFinder.findSecret(content) // Verify the TOTP secret is valid and disable TOTP if not.
val secret = totpFinder.findSecret(content)
totpSecret =
if (secret != null && calculateTotp(secret).isOk) {
secret
} else {
null
}
} }
public fun hasTotp(): Boolean { public fun hasTotp(): Boolean {
@ -175,26 +193,21 @@ constructor(
return null return null
} }
private fun calculateTotp(): Totp { private fun calculateTotp(secret: String = totpSecret!!): Result<Totp, Throwable> {
val digits = totpFinder.findDigits(content) val digits = totpFinder.findDigits(content)
val totpPeriod = totpFinder.findPeriod(content) val totpPeriod = totpFinder.findPeriod(content)
val totpAlgorithm = totpFinder.findAlgorithm(content) val totpAlgorithm = totpFinder.findAlgorithm(content)
val issuer = totpFinder.findIssuer(content) val issuer = totpFinder.findIssuer(content)
val millis = clock.millis() val millis = clock.millis()
val remainingTime = (totpPeriod - ((millis / THOUSAND_MILLIS) % totpPeriod)).seconds val remainingTime = (totpPeriod - ((millis / THOUSAND_MILLIS) % totpPeriod)).seconds
Otp.calculateCode( return Otp.calculateCode(
totpSecret!!, secret,
millis / (THOUSAND_MILLIS * totpPeriod), millis / (THOUSAND_MILLIS * totpPeriod),
totpAlgorithm, totpAlgorithm,
digits, digits,
issuer issuer
) )
.mapBoth( .mapBoth({ code -> Ok(Totp(code, remainingTime)) }, ::Err)
{ code ->
return Totp(code, remainingTime)
},
{ throwable -> throw throwable }
)
} }
@AssistedFactory @AssistedFactory

View file

@ -13,6 +13,7 @@ import app.passwordstore.util.totp.UriTotpFinder
import java.util.Locale import java.util.Locale
import kotlin.test.Test import kotlin.test.Test
import kotlin.test.assertEquals import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertNotNull import kotlin.test.assertNotNull
import kotlin.test.assertNull import kotlin.test.assertNull
import kotlin.test.assertTrue import kotlin.test.assertTrue
@ -181,6 +182,13 @@ class PasswordEntryTest {
} }
} }
// https://github.com/android-password-store/Android-Password-Store/issues/2949
@Test
fun disablesTotpForInvalidUri() = runTest {
val entry = makeEntry("password\notpauth://totp/otp-secret?secret=")
assertFalse(entry.hasTotp())
}
@Test @Test
fun onlyLooksForUriInFirstLine() { fun onlyLooksForUriInFirstLine() {
val entry = makeEntry("id:\n$TOTP_URI") val entry = makeEntry("id:\n$TOTP_URI")