| # | Finding | Severity | Status | Reference | File(s) |
|---|---|---|---|---|---|
| 1 | Upload key passphrase stored in plaintext in keystore.properties | Low | Deferred | CWE-522, OWASP M9 | keystore.properties |
| 2 | HMAC verification intentionally non-fatal — cross-device migration design trade-off | Info | Design Decision | — | BackupManager.kt:203 |
| 3 | Non-constant-time comparison for SHA-256 checksum and HMAC tag | High | Fixed | CWE-208, OWASP M6 | BackupManager.kt:196, BackupKeyStore.kt:56 |
| 4 | Backup file is unencrypted by design — pre-export disclosure added | Info | Design Decision | — | SettingsScreen.kt |
| 5 | Notification lock screen visibility not explicitly configured | Medium | Fixed | CWE-200, OWASP M1, MASTG MSTG-STORAGE-11 | NotificationHelper.kt:185 |
| 6 | Unvalidated integer extras in notification receivers — integer overflow | Medium | Fixed | CWE-190, OWASP M4 | NotificationActionReceiver.kt:27 |
| 7 | Backup validation omits settings integer range checks — DoS/silent misconfiguration | Medium | Fixed | CWE-20, OWASP M4 | BackupManager.kt:310 |
| 8 | android.util.Log calls present in release builds without DEBUG guard | Low | Fixed | CWE-532, OWASP M9 | Multiple files |
| 9 | Database passphrase ByteArray not zeroed after use | Low | Fixed | CWE-316, MASTG MSTG-CRYPTO-6 | DatabaseEncryptionManager.kt:103 |
| 10 | Keystore keys have no user authentication requirement | Low | Accepted | CWE-522, OWASP M3, MASTG MSTG-CRYPTO-5 | DatabaseEncryptionManager.kt:138, BackupKeyStore.kt:87 |
| 11 | eventId.toInt() without bitmask in NotificationCleanupCoordinator | Low | Fixed | CWE-190 | NotificationCleanupCoordinator.kt:43 |
| 12 | Legacy DataStore deletion non-atomic with no error handling | Low | Accepted | CWE-362 | RoomSettingsRepository.kt:332 |
| 13 | HMAC verification occurs outside Keystore (by design — document it) | Info | — | — | BackupKeyStore.kt |
| 14 | android:allowBackup="false" correctly set | Info | — | — | AndroidManifest.xml |
| 15 | No network code or cleartext traffic present | Info | — | OWASP M3 | — |
| 16 | BootReceiver exported=true (required, protected broadcast) | Info | — | — | AndroidManifest.xml |
| 17 | No deep links or exported URI handlers | Info | — | — | AndroidManifest.xml |
| 18 | Room schema files exported to project directory | Info | — | — | build.gradle.kts:100 |
The file keystore.properties stores the keystore password, key password, and key alias in plaintext. The file is correctly excluded from version control via .gitignore and has never been committed to the repository.
Importantly, this project uses Google Play App Signing: the local dailycue-release.keystore is the upload key only. Google holds the actual distribution signing key and uses it to re-sign AABs before serving them to users. This significantly reduces the risk compared to a self-signed distribution scenario:
Note: for local development (debug APK testing), keystore.properties is not consulted at all — debug builds use Android's auto-generated ~/.android/debug.keystore. The file is only needed when building signed release AABs for Play Store upload.
signingProp() / findProperty() fallback already in build.gradle.kts. This eliminates the plaintext file from the developer workstation entirely for CI-driven release builds.build.gradle.kts via a shell invocation at configuration time.chmod 600 keystore.properties).The HMAC-SHA256 authenticity check logs a warning on mismatch but allows the import to continue. This is a deliberate design decision required to support cross-device migration — the primary use case for backup/restore.
The HMAC key is generated and stored in the Android Keystore with hardware-backed protection. It is device-specific and non-exportable by construction: there is no mechanism to transfer it to a new device. Therefore, any backup imported on a new device will always fail the HMAC check. Making the check fatal would silently break every cross-device migration.
envelope.hmac?.let { expectedTag ->
if (!backupKeyStore.verify(payloadBytes, expectedTag)) {
Log.w(TAG, "Backup HMAC mismatch — file was not created by this device.")
// Non-fatal by design: HMAC key is device-bound (Android Keystore).
// A mismatching HMAC on a new device is the expected case, not an attack.
}
}
The SHA-256 checksum (enforced — import aborts on mismatch) still protects against accidental file corruption. The HMAC provides same-device provenance only; its mismatch on a new device is expected and correct.
Residual risk: a crafted backup (forged SHA-256 + malicious payload) can be imported. This requires the attacker to construct a valid JSON file and socially engineer the user into importing it — a meaningful but accepted threat for a local-only, offline application with no network surface.
Both the SHA-256 checksum and the HMAC tag are compared using standard Kotlin string equality (!= / ==), which may short-circuit on the first mismatching byte, leaking timing information that enables iterative recovery of the expected value. While a direct timing oracle is harder to exploit on a local device than over a network, using constant-time comparison is a trivially achievable correctness requirement for any cryptographic authentication primitive.
Use MessageDigest.isEqual(byte[], byte[]) for all security-sensitive comparisons:
// SHA-256 hex checksum:
if (!MessageDigest.isEqual(
actualChecksum.toByteArray(Charsets.UTF_8),
expectedChecksum.toByteArray(Charsets.UTF_8))) { ... }
// HMAC Base64 tag:
return MessageDigest.isEqual(
Base64.decode(actualTag, Base64.NO_WRAP),
Base64.decode(expectedTag, Base64.NO_WRAP)
)
BackupManager.kt and BackupKeyStore.kt updated to use MessageDigest.isEqual() for all HMAC and SHA-256 checksum comparisons, eliminating the timing side-channel.
The backup export writes a plaintext JSON file containing all schedule data, including caregiver contact names and phone numbers. The SQLCipher database encryption is on-device only and does not extend to exported files.
This is a deliberate design decision to maximise cross-device migration fidelity. Encrypting the backup with a device-bound Keystore key would prevent restoration on a new device (the same constraint that makes the HMAC non-fatal). Encrypting with a user passphrase would require users to manage and remember a separate credential, which is in tension with the application's accessibility goals.
Mitigation implemented: a pre-export disclosure dialog was added to SettingsScreen.kt. When the user taps "Backup Data", an AlertDialog now informs them that the file will contain contact names and phone numbers in readable form, and requires explicit confirmation before the file picker opens. The export can only proceed after the user taps "Export anyway".
Payload encryption (AES-256-GCM with user passphrase) remains an option for a future hardening sprint if the threat model evolves.
The NotificationCompat.Builder does not call setVisibility(), leaving lock screen behavior dependent on Android version defaults and user settings. Event titles and routine names in notification content may reveal sensitive scheduling information (e.g., medical appointments, therapy sessions, personal habits) to anyone who can view the device's lock screen or notification shade — a risk that is especially relevant for the neurodivergent user base this application targets.
// On notification channel creation: channel.lockscreenVisibility = Notification.VISIBILITY_PRIVATE // On the notification builder: .setVisibility(NotificationCompat.VISIBILITY_PRIVATE) // VISIBILITY_PRIVATE shows a generic placeholder on the lock screen // and reveals full content only after device unlock.
Consider providing a user-configurable setting for lock screen notification detail level.
NotificationHelper.kt: lockscreenVisibility = Notification.VISIBILITY_PRIVATE added to channel creation; .setVisibility(NotificationCompat.VISIBILITY_PRIVATE) added to the notification builder.
The snoozeMinutes value from the intent is used without bounds checking in an Int × Int multiplication that silently overflows for large values, widening to an incorrect Long delay. Additionally, NotificationCleanupCoordinator.kt:43 uses a bare eventId.toInt() without the and 0x7FFFFFFF mask applied elsewhere, causing PendingIntent alarm cancellation to silently fail for large event IDs.
val snoozeMinutes = intent.getIntExtra("snoozeMinutes", 7)
val delayMillis = snoozeMinutes * 60 * 1000L // Int × Int overflows at ~35M minutes
// Clamp before arithmetic:
val snoozeMinutes = intent.getIntExtra("snoozeMinutes", 7).coerceIn(1, 60)
val delayMillis = snoozeMinutes.toLong() * 60 * 1000L
// NotificationCleanupCoordinator — consistent mask:
val requestCode = (eventId and 0x7FFFFFFF).toInt()
NotificationActionReceiver.kt: coerceIn(1, 60) and .toLong() widening applied. NotificationCleanupCoordinator.kt: and 0x7FFFFFFF bitmask added before .toInt() cast.
The validateBackupPayload() method validates routines, events, and checklist items but performs no range validation on the settings section. Combined with Finding 2 (non-fatal HMAC), a crafted backup could set routineScheduleDays = Int.MAX_VALUE to trigger excessive future-event generation (denial-of-service), or set notificationMinutesBefore to a negative value to silently disable all notifications without the user's awareness.
payload.settings?.let { s ->
require(s.reminderDurationMinutes in 1..480) { "Invalid reminder duration" }
require(s.routineDurationMinutes in 1..480) { "Invalid routine duration" }
require(s.routineScheduleDays in 1..365) { "Invalid schedule days" }
require(s.notificationMinutesBefore in 0..480) { "Invalid notification lead time" }
}
BackupManager.kt: range checks for all four settings integers added to validateBackupPayload().
All android.util.Log calls are unconditional — none are gated on BuildConfig.DEBUG. R8/ProGuard does not strip log calls by default. Log messages visible in release builds include encryption migration details, backup HMAC mismatch status, SMS contact counts, and internal exception details. On rooted devices or via ADB, these are readable by any user with shell access and aid reverse engineering efforts.
Add ProGuard/R8 rules to strip all android.util.Log calls in release, or adopt the Timber logging facade configured to suppress logs in release builds:
-assumenosideeffects class android.util.Log {
public static boolean isLoggable(java.lang.String, int);
public static int v(...);
public static int d(...);
public static int i(...);
public static int w(...);
public static int e(...);
}
proguard-rules.pro: -assumenosideeffects block added stripping all android.util.Log methods (v, d, i, w, e, isLoggable) from release builds via R8.
getOrCreatePassphrase() returns a ByteArray containing the 256-bit database passphrase that is passed to SupportOpenHelperFactory but never zeroed in the calling scope. The array may remain in the JVM heap for minutes after the last reference is dropped. On a rooted device or via memory dump, the plaintext passphrase could be recovered from the heap, bypassing SQLCipher encryption entirely.
val passphrase = getOrCreatePassphrase()
return try {
SupportOpenHelperFactory(passphrase.copyOf()) // factory gets its own copy
} finally {
passphrase.fill(0) // zero our local copy immediately
}
Important: SupportOpenHelperFactory stores a reference to the passphrase ByteArray internally — it does not make a defensive copy. Zeroing the array directly would wipe the key before SQLCipher opens the database, causing SQLiteNotADatabaseException on every launch. The correct pattern passes passphrase.copyOf() so the factory retains a valid key, then zeros the local variable.
DatabaseEncryptionManager.kt: getSupportFactory() passes passphrase.copyOf() to SupportOpenHelperFactory and zeros the local variable in finally. The initial fix (fd8683c) zeroed the array directly, which wiped the factory's reference and caused SQLiteNotADatabaseException on launch — discovered and corrected in commit 22480da.
Neither the database encryption key nor the backup HMAC key specifies setUserAuthenticationRequired(true). Any code running as the app's process UID can use these keys without PIN, password, or biometric. On a stolen unlocked device or on a rooted device where another process has escalated to the app's UID, the Keystore keys are accessible without additional friction.
// Per-session authentication with 30-minute validity window: .setUserAuthenticationRequired(true) .setUserAuthenticationValidityDurationSeconds(30 * 60)
If requiring authentication per session is deemed too disruptive, document the decision explicitly as an accepted risk in the threat model.
The cancelAlarm() method uses a bare eventId.toInt() as the PendingIntent request code without the and 0x7FFFFFFF mask applied consistently elsewhere in the notification stack (NotificationHelper.kt lines 162, 179, 228). For event IDs larger than Int.MAX_VALUE, the resulting negative request code differs from the one used to create the alarm, causing alarm cancellation to silently fail and leaving stale alarms scheduled in WorkManager.
// Apply the same mask used throughout NotificationHelper: val requestCode = (eventId and 0x7FFFFFFF).toInt()
NotificationCleanupCoordinator.kt:43: eventId.toInt() replaced with (eventId and 0x7FFFFFFF).toInt(), matching the pattern used in NotificationHelper.kt.
The deleteDataStoreFiles() method deletes DataStore files one-by-one with no error handling. If the app is killed mid-operation, partial DataStore data (including the owner name) survives on disk in an unencrypted file. A subsequent launch skips migration because the Room record already exists, leaving the stale DataStore files as abandoned unencrypted PII storage with no automatic cleanup.
| # | Observation | Assessment |
|---|---|---|
| 13 | HMAC verification occurs outside the Keystore (caller compares bytes) | By design — Android Keystore API requires this for HMAC-SHA256. Document explicitly so future reviewers understand the intent. No code change required. |
| 14 | android:allowBackup="false" in AndroidManifest | Correctly set. Prevents Android Auto Backup from syncing the encrypted database, DataStore, and SharedPreferences to Google Drive. |
| 15 | No network libraries or HTTP/HTTPS code present | Verified across all source files. The app operates entirely offline. OWASP M3 (Insecure Communication) does not apply. |
| 16 | BootReceiver is android:exported="true" | Required to receive BOOT_COMPLETED. This is a protected broadcast on API 28+ — only the OS can send it. No remediation needed. |
| 17 | No deep links, custom URI schemes, or exported URI handlers | MainActivity has only a MAIN/LAUNCHER intent filter. No deep-link attack surface is present. |
| 18 | Room schema JSON files exported to app/schemas/ | Build-time artifact only — not bundled in the APK. Useful for migration verification. Consider gitignoring if schema detail is deemed sensitive information. |
android:exported="false" — prevents external apps from sending crafted intents to notification action and alarm handlers.isMinifyEnabled = true, isShrinkResources = true), reducing attack surface and impeding reverse engineering.PendingIntent.FLAG_IMMUTABLE used throughout — prevents intent mutation attacks on Android 12+ (API 31+).@Query bindings — no raw SQL string concatenation; SQL injection is structurally impossible via Room's annotation processor.Intent.ACTION_SENDTO (system SMS app) — the app never requires the SEND_SMS permission; users retain full control of every message send.SendRoutineCompletionSmsUseCase strips injection characters (;, @, :) before constructing the smsto: URI.SecureRandom used for passphrase generation — not java.util.Random; 256-bit key space is cryptographically sound.fallbackToDestructiveMigration(BuildConfig.DEBUG) — destructive Room migration is permitted only in debug builds, protecting production user data during schema changes.