Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions app/src/main/java/com/eatssu/android/di/SecurePrefsModule.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package com.eatssu.android.di

import android.content.Context
import android.content.SharedPreferences
import com.google.firebase.crashlytics.FirebaseCrashlytics
import timber.log.Timber
import dagger.Module
import dagger.Provides
import dagger.hilt.InstallIn
Expand All @@ -16,6 +18,21 @@ object SecurePrefsModule {
@Provides
@Singleton
fun provideSecurePrefs(@ApplicationContext context: Context): SharedPreferences {
return try {
createSecurePrefs(context)
} catch (e: Exception) {
Timber.e(e, "Failed to create SecurePrefs. Resetting...")
FirebaseCrashlytics.getInstance().recordException(e)

// Clear data
context.deleteSharedPreferences("secure_prefs")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

하드코딩된 문자열 "secure_prefs"가 28라인과 41라인 두 곳에서 사용되고 있습니다. 이 문자열을 object 내에 private const val로 선언하여 재사용하면 오타 발생 가능성을 줄이고 유지보수성을 높일 수 있습니다.

예시:

object SecurePrefsModule {
    private const val SECURE_PREFS_FILENAME = "secure_prefs"

    // ...
}


// Retry
createSecurePrefs(context)
}
Comment on lines +23 to +32
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

catch 블록에는 두 가지 잠재적인 문제가 있습니다.

  1. 재시도 실패 시 앱 충돌 (심각도: Critical): 31번째 줄의 createSecurePrefs(context) 재시도가 실패할 경우, 해당 예외가 처리되지 않아 앱이 충돌하게 됩니다. 이는 핫픽스의 주 목적인 '앱 충돌 방지'에 위배됩니다. 재시도 로직을 try-catch로 한번 더 감싸서, 두 번째 실패 시에는 명확한 예외를 던지도록 처리해야 합니다.

  2. 일반적인 Exception 처리 (심각도: High): KeyStoreException과 같은 특정 예외가 아닌 일반 Exception을 잡게 되면, 예상치 못한 다른 종류의 예외(예: NullPointerException)까지 이 복구 로직을 타게 되어 의도치 않은 데이터 삭제를 유발할 수 있습니다. when 절을 사용해 GeneralSecurityException이나 IOException 같은 예상된 예외에 대해서만 복구 로직을 수행하고, 그 외에는 예외를 다시 던져서 문제를 파악할 수 있도록 하는 것이 더 안전합니다.

아래는 이 두 가지 문제를 모두 수정한 제안 코드입니다. 참고로, 제안된 코드를 적용하려면 java.io.IOExceptionjava.security.GeneralSecurityException을 import해야 할 수 있습니다.

        } catch (e: Exception) {
            when (e) {
                is java.security.GeneralSecurityException, is java.io.IOException -> {
                    Timber.e(e, "Failed to create SecurePrefs. Resetting...")
                    FirebaseCrashlytics.getInstance().recordException(e)

                    // Clear data
                    context.deleteSharedPreferences("secure_prefs")

                    // Retry
                    try {
                        createSecurePrefs(context)
                    } catch (retryException: Exception) {
                        Timber.e(retryException, "Failed to create SecurePrefs even after reset.")
                        FirebaseCrashlytics.getInstance().recordException(retryException)
                        throw IllegalStateException("Could not initialize SecurePrefs after reset.", retryException)
                    }
                }

                else -> throw e
            }
        }

}

private fun createSecurePrefs(context: Context): SharedPreferences {
val masterKey = androidx.security.crypto.MasterKey.Builder(context)
.setKeyScheme(androidx.security.crypto.MasterKey.KeyScheme.AES256_GCM)
.build()
Expand Down