Conversation
- 디바이스 루팅 여부 확인 기능 구현 (test-keys, 루팅 바이너리, su 명령어 탐지 방식) - Chrome 브라우저 설치 여부 확인 기능 추가
- KakaoLoginSdk에 보안 점검 로직 추가 - Chrome 브라우저 미설치 시 예외 발생 - 루팅된 기기에서 로그인 시도 시 차단
- 허용된 도메인 외 요청은 외부 브라우저로 열도록 처리 - SSL 인증서 오류 발생 시 로딩 차단으로 보안 강화
- market/web intent URI prefix를 상수로 분리해 가독성 향상 - Google Play 앱 패키지명을 명시하여 의도된 마켓 앱으로 이동하도록 지정
WalkthroughThis update introduces enhanced security measures for login and WebView features. It adds root and Chrome checks before Kakao login, centralizes security checks via a new interface and implementation, and restricts WebView navigation to allowed domains with SSL error blocking. Navigation and URL handling are refactored for greater safety and modularity. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant KakaoLoginSdk
participant LoginSecurityChecker
User->>App: Initiate Kakao Login
App->>KakaoLoginSdk: login(context)
KakaoLoginSdk->>LoginSecurityChecker: isChromeInstalled(context)
alt Chrome not installed
KakaoLoginSdk-->>App: Throw Chrome required error
else Chrome installed
KakaoLoginSdk->>LoginSecurityChecker: isDeviceRooted()
alt Device rooted
KakaoLoginSdk-->>App: Throw rooted device error
else Not rooted
KakaoLoginSdk->>App: Proceed with Kakao login flow
end
end
sequenceDiagram
participant User
participant App
participant WebViewScreen
participant SecureWebViewClient
User->>App: Request to open URL in WebView
App->>WebViewScreen: Load URL
WebViewScreen->>SecureWebViewClient: shouldOverrideUrlLoading(request)
alt URL host matches allowed domains
SecureWebViewClient-->>WebViewScreen: Allow internal load
else
SecureWebViewClient-->>App: Launch external intent
end
SecureWebViewClient->>WebViewScreen: onReceivedSslError
alt SSL Error
SecureWebViewClient-->>WebViewScreen: Cancel load
end
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
MoonsuKang
changed the base branch from
develop
to
refactor/#283-extract-strings-i18n
SYAAINN
left a comment
SYAAINN
left a comment
There was a problem hiding this comment.
난이도가 꽤 있긴한데 재밌네요 .. 수고하셨습니다!
아마 이번 스프린트때 구글로그인 구현하면서 다시 한번 보게될 아이들같아서 일단 어프찍고, 더 공부해보겠습니다~
| @Binds | ||
| abstract fun bindLoginSecurityChecker( | ||
| impl: DefaultLoginSecurityChecker, | ||
| ): LoginSecurityChecker |
There was a problem hiding this comment.
[p3]
@singleton 어노테이션은 필요없을까요? 매번 새로운 객체가 생성되어야할 필요가 있는지 궁금합니다.
Hilt가 하나의 인스턴스를 재사용함으로써 의도치 않은 버그를 방지하거나, 메모리 효율을 챙길 수 있어 제외해야 하는 경우가 아니라면 붙이는게 좋지 않을까라고 생각하는데, @singleton 어노테이션에 대한 의견이 궁금합니다 ㅎ
There was a problem hiding this comment.
DefaultLoginSecurityChecker에 싱글톤 선언을 해서 Modyle에는 바인딩만 했습니다.
요래도 DI 트리를 그려나가면서 인스턴스 하나만 생성이 되긴하는데 일관성을 위해서 Module단으로 옮길게용
MoonsuKang
changed the base branch from
refactor/#283-extract-strings-i18n
to
develop
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
app/src/main/java/com/sopt/clody/core/security/login/SecurityModule.kt (1)
13-18: Proper dependency injection setup.The module correctly binds the interface to its implementation with appropriate singleton scope. The
@Singletonannotation on the binding method addresses the previous review feedback about ensuring single instance creation.
🧹 Nitpick comments (2)
app/src/main/java/com/sopt/clody/core/security/login/DefaultLoginSecurityChecker.kt (1)
43-44: Address static analysis warnings about exception swallowing.While the current exception handling is intentional for security checks, consider logging for debugging purposes.
} catch (e: Exception) { + // Log for debugging in debug builds only + if (BuildConfig.DEBUG) { + android.util.Log.d("SecurityChecker", "Root detection check failed", e) + } false }} catch (e: PackageManager.NameNotFoundException) { + // Expected exception when Chrome is not installed false }Also applies to: 58-59
app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpScreen.kt (1)
33-34: Address the TODO comment for complete error handling.The TODO comment indicates incomplete error message handling. In a security-focused PR, proper error handling is crucial to prevent information leakage and ensure robust security checks.
Would you like me to help implement the Snackbar or Dialog error message handling, or should this be tracked in a separate issue?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
app/build.gradle.kts(1 hunks)app/src/main/java/com/sopt/clody/core/login/KakaoLoginSdk.kt(1 hunks)app/src/main/java/com/sopt/clody/core/security/login/DefaultLoginSecurityChecker.kt(1 hunks)app/src/main/java/com/sopt/clody/core/security/login/LoginSecurityChecker.kt(1 hunks)app/src/main/java/com/sopt/clody/core/security/login/SecurityModule.kt(1 hunks)app/src/main/java/com/sopt/clody/core/security/weview/SecureWebViewClient.kt(1 hunks)app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpContract.kt(1 hunks)app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpScreen.kt(3 hunks)app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpViewModel.kt(2 hunks)app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/navigation/SignUpNavigation.kt(1 hunks)app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/page/TermsOfServicePage.kt(3 hunks)app/src/main/java/com/sopt/clody/presentation/ui/main/ClodyNavHost.kt(2 hunks)app/src/main/java/com/sopt/clody/presentation/ui/setting/navigation/SettingNavigation.kt(0 hunks)app/src/main/java/com/sopt/clody/presentation/ui/setting/screen/SettingScreen.kt(3 hunks)app/src/main/java/com/sopt/clody/presentation/ui/webview/WebViewNavGraph.kt(1 hunks)app/src/main/java/com/sopt/clody/presentation/ui/webview/WebViewScreen.kt(3 hunks)app/src/main/java/com/sopt/clody/presentation/utils/appupdate/AppUpdateUtils.kt(1 hunks)app/src/main/res/values/strings.xml(1 hunks)
💤 Files with no reviewable changes (1)
- app/src/main/java/com/sopt/clody/presentation/ui/setting/navigation/SettingNavigation.kt
🧰 Additional context used
🧬 Code Graph Analysis (5)
app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/navigation/SignUpNavigation.kt (1)
app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpScreen.kt (1)
SignUpRoute(16-56)
app/src/main/java/com/sopt/clody/presentation/ui/webview/WebViewNavGraph.kt (1)
app/src/main/java/com/sopt/clody/presentation/ui/webview/WebViewScreen.kt (1)
WebViewRoute(22-31)
app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/page/TermsOfServicePage.kt (1)
app/src/main/java/com/sopt/clody/presentation/ui/setting/navigation/SettingNavigation.kt (1)
navigateToWebView(66-71)
app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpScreen.kt (1)
app/src/main/java/com/sopt/clody/presentation/ui/setting/navigation/SettingNavigation.kt (1)
navigateToWebView(66-71)
app/src/main/java/com/sopt/clody/presentation/ui/setting/screen/SettingScreen.kt (4)
app/src/main/java/com/sopt/clody/presentation/ui/setting/navigation/SettingNavigation.kt (1)
navigateToWebView(66-71)app/src/main/java/com/sopt/clody/presentation/ui/setting/component/SettingTopAppBar.kt (1)
SettingTopAppBar(15-38)app/src/main/java/com/sopt/clody/presentation/ui/setting/component/SettingOption.kt (1)
SettingOption(21-51)app/src/main/java/com/sopt/clody/presentation/ui/setting/component/SettingSeparateLine.kt (1)
SettingSeparateLine(12-20)
🪛 detekt (1.23.8)
app/src/main/java/com/sopt/clody/core/security/login/DefaultLoginSecurityChecker.kt
[warning] 43-43: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 58-58: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (30)
app/build.gradle.kts (1)
34-40: LGTM! Secure and configurable WebView domain restriction.The build configuration properly centralizes allowed WebView domains with sensible defaults and configurable overrides. This security enhancement aligns well with the PR's objective to strengthen WebView security.
app/src/main/res/values/strings.xml (1)
219-220: LGTM! Clear security error messages.The new localized error strings effectively communicate login security restrictions to users. The messages clearly explain Chrome installation requirements and rooted device restrictions, supporting the enhanced security measures.
app/src/main/java/com/sopt/clody/presentation/ui/main/ClodyNavHost.kt (2)
34-34: LGTM! Improved module separation.Moving WebView functionality to a dedicated module improves code organization and aligns with the security-focused WebView enhancements.
76-76: LGTM! Consistent navigation pattern.The addition of
navigateToWebViewparameter follows the established navigation callback pattern used throughout the app and enables secure WebView access from the signup flow.app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpContract.kt (1)
36-36: LGTM! Standard MVI pattern implementation.The new
OpenWebViewintent andNavigateToWebViewside effect follow established MVI patterns and enable secure WebView navigation from the signup flow. URL validation is appropriately handled by the WebView implementation rather than at the contract level.Also applies to: 42-42
app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpViewModel.kt (2)
60-60: LGTM! Consistent intent handling.The
OpenWebViewintent handling follows the established pattern and integrates cleanly with the existing intent switch structure.
106-108: LGTM! Clean implementation.The
handleOpenWebViewfunction is appropriately simple and follows the established pattern of other intent handlers. The direct forwarding of the URL via side effect is the correct approach for this navigation use case.app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/navigation/SignUpNavigation.kt (1)
13-13: LGTM! Clean navigation parameter addition.The new
navigateToWebViewparameter is properly integrated into the navigation graph and aligns with the broader WebView navigation refactoring described in the PR objectives.Also applies to: 19-19
app/src/main/java/com/sopt/clody/core/security/login/LoginSecurityChecker.kt (1)
5-8: Well-designed security interface.The interface clearly defines the contract for login security checks with descriptive method names and appropriate signatures. This provides a clean abstraction for device security validation.
app/src/main/java/com/sopt/clody/presentation/ui/webview/WebViewNavGraph.kt (1)
11-18: Clean navigation graph implementation.The navigation graph correctly extracts the encoded URL parameter using the modern
toRoute<Route.WebView>()approach and properly passes it to theWebViewRoutecomposable along with the back navigation callback.app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/page/TermsOfServicePage.kt (2)
45-45: Excellent separation of concerns refactoring.The composable now properly delegates navigation responsibility to its caller through the
navigateToWebViewparameter, making it more testable and following the single responsibility principle. The direct invocation of the navigation callback with URL constants is clean and maintainable.Also applies to: 115-115, 122-122
172-172: Preview properly updated.The preview composable correctly includes the new
navigateToWebViewparameter with an empty lambda, maintaining preview functionality.app/src/main/java/com/sopt/clody/presentation/utils/appupdate/AppUpdateUtils.kt (3)
11-13: LGTM! Good security practice with explicit package targeting.Adding constants for the Play Store package and URI prefixes improves maintainability and security by ensuring intents target the correct application.
22-25: Excellent security enhancement with explicit package setting.Explicitly setting the package to
com.android.vendingprevents potential intent hijacking by malicious apps that might try to intercept market navigation intents.
31-41: Well-implemented error handling with proper fallback logic.The
runCatchingapproach provides safe execution with appropriate fallback to web browser and logging for debugging. The comment about the extreme case scenario is helpful.app/src/main/java/com/sopt/clody/presentation/ui/webview/WebViewScreen.kt (3)
39-41: Good practice with URL decoding.Decoding the URL before loading prevents potential issues with encoded URLs and ensures the WebView receives the proper URL format.
54-54: Security enhancement with custom WebViewClient.Using
SecureWebViewClientwith domain restrictions is a good security practice to prevent unauthorized navigation.
60-64: Excellent WebView security hardening.These security settings significantly improve WebView security:
allowFileAccess = falseprevents file system accessallowContentAccess = falseblocks content provider accessjavaScriptCanOpenWindowsAutomatically = falseprevents popup attacksMIXED_CONTENT_NEVER_ALLOWblocks mixed HTTP/HTTPS contentapp/src/main/java/com/sopt/clody/core/security/weview/SecureWebViewClient.kt (1)
30-32: Good SSL error handling.Blocking all SSL errors by calling
handler.cancel()is a good security practice that prevents man-in-the-middle attacks.app/src/main/java/com/sopt/clody/core/login/KakaoLoginSdk.kt (2)
18-20: Good dependency injection pattern for security checks.Injecting the
LoginSecurityCheckerenables testability and follows proper dependency inversion principles.
22-28: Effective security gating for login flow.The pre-login security checks are well-implemented:
- Chrome installation check ensures web-based Kakao login compatibility
- Root detection prevents login on compromised devices
- Localized error messages improve user experience
app/src/main/java/com/sopt/clody/core/security/login/DefaultLoginSecurityChecker.kt (3)
28-29: Good first-level root detection.Checking
Build.TAGSfor "test-keys" is a standard and effective method for detecting development/debug builds that are typically rooted.
31-38: Comprehensive file-based root detection.The path checking covers common locations for root binaries and apps, providing good coverage for detecting rooted devices.
55-60: Correct Chrome detection implementation.Using
PackageManager.getPackageInfo()is the standard and reliable method for checking if a specific package is installed.app/src/main/java/com/sopt/clody/presentation/ui/setting/screen/SettingScreen.kt (3)
44-47: Well-implemented navigation refactoring for security enhancement.The refactoring to use callback-based navigation with predefined URL constants is a solid architectural improvement that supports the PR's security objectives. This centralizes WebView navigation and removes direct intent handling from the UI layer.
58-61: Clean function signature update.The function signature update to accept lambda parameters instead of handling URLs internally is consistent with the navigation refactoring and promotes better separation of concerns.
79-85: Consistent callback usage across setting options.The setting options now properly delegate navigation through the provided callbacks, maintaining consistency with the new navigation pattern.
app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpScreen.kt (3)
21-21: Good addition of WebView navigation parameter.The addition of
navigateToWebViewparameter to the route function supports the centralized navigation pattern and security enhancements outlined in the PR.
36-38: Proper side effect handling for WebView navigation.The NavigateToWebView side effect is correctly handled by calling the navigation callback with the provided URL.
76-78: Consistent callback delegation pattern.The WebView navigation callback is properly passed to TermsOfServicePage, maintaining consistency with the navigation refactoring pattern used throughout the codebase.
| var webView: WebView? by remember { mutableStateOf(null) } | ||
| val canGoBack by remember { derivedStateOf { webView?.canGoBack() ?: false } } | ||
|
|
||
| val allowedDomains = BuildConfig.ALLOWED_WEBVIEW_DOMAINS.split(",").map { it.trim() } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the build config field exists and fallback handling.
The code relies on BuildConfig.ALLOWED_WEBVIEW_DOMAINS but there's no fallback if this field is empty or malformed.
🏁 Script executed:
#!/bin/bash
# Description: Check if ALLOWED_WEBVIEW_DOMAINS is properly defined in build configuration
# Search for the build config field definition
rg -A 3 -B 3 "ALLOWED_WEBVIEW_DOMAINS" --type gradleLength of output: 201
I couldn’t find any Gradle definitions for ALLOWED_WEBVIEW_DOMAINS with the previous search. Let’s look specifically in your build scripts:
🏁 Script executed:
#!/bin/bash
# Search for ALLOWED_WEBVIEW_DOMAINS in all Gradle build files
# 1. Look for a buildConfigField declaration
rg -n "buildConfigField.*ALLOWED_WEBVIEW_DOMAINS" -g "*.gradle*"
# 2. Fallback: search for any mention in Gradle files
rg -n "ALLOWED_WEBVIEW_DOMAINS" -g "*.gradle*"Length of output: 331
Check for empty or malformed ALLOWED_WEBVIEW_DOMAINS and add a fallback
We’ve confirmed that ALLOWED_WEBVIEW_DOMAINS is defined in app/build.gradle.kts (line 40). However, in WebViewScreen.kt the code unconditionally does:
val allowedDomains = BuildConfig.ALLOWED_WEBVIEW_DOMAINS
.split(",")
.map { it.trim() }If the string is blank or malformed you’ll end up with [""] or unexpected entries.
• File: app/src/main/java/com/sopt/clody/presentation/ui/webview/WebViewScreen.kt
Line: 46
Suggested change (example):
- val allowedDomains = BuildConfig.ALLOWED_WEBVIEW_DOMAINS
- .split(",")
- .map { it.trim() }
+ val allowedDomains = BuildConfig.ALLOWED_WEBVIEW_DOMAINS
+ .takeIf { it.isNotBlank() }
+ ?.split(",")
+ ?.map { it.trim() }
+ ?.filter { it.isNotEmpty() }
+ ?: emptyList()This ensures you never end up with blank entries and have a safe default if the config is empty or invalid.
🤖 Prompt for AI Agents
In app/src/main/java/com/sopt/clody/presentation/ui/webview/WebViewScreen.kt at
line 46, the code splits ALLOWED_WEBVIEW_DOMAINS without checking if it is empty
or malformed, which can result in a list with blank entries. To fix this, add a
check to verify if ALLOWED_WEBVIEW_DOMAINS is null, blank, or malformed before
splitting; if it is, assign a safe default list of allowed domains. Also, filter
out any blank entries after splitting and trimming to ensure the list contains
only valid domain strings.
| val host = request.url.host ?: return true | ||
| val isSafeDomain = allowedDomains.any { host.contains(it) } |
There was a problem hiding this comment.
Potential security bypass with substring matching.
The current implementation uses host.contains(it) for domain checking, which could allow bypasses. For example, evil-notion.so.malicious.com would match notion.so.
Consider using exact domain matching or suffix checking:
- val isSafeDomain = allowedDomains.any { host.contains(it) }
+ val isSafeDomain = allowedDomains.any { allowedDomain ->
+ host == allowedDomain || host.endsWith(".$allowedDomain")
+ }🤖 Prompt for AI Agents
In app/src/main/java/com/sopt/clody/core/security/weview/SecureWebViewClient.kt
around lines 17 to 18, the domain check uses substring matching with
host.contains(it), which can be bypassed by malicious subdomains. Replace this
with exact domain matching or check if the host ends with the allowed domain
preceded by a dot to ensure only valid subdomains or the exact domain are
matched, preventing security bypasses.
| Intent(Intent.ACTION_VIEW, request.url).let { | ||
| context.startActivity(it) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential security risk with unverified intent launching.
Launching intents without verification could be exploited by malicious URLs (e.g., intent:// schemes or file URLs).
Add intent verification and safe launching:
- Intent(Intent.ACTION_VIEW, request.url).let {
- context.startActivity(it)
- }
+ Intent(Intent.ACTION_VIEW, request.url).let { intent ->
+ if (intent.resolveActivity(context.packageManager) != null &&
+ (request.url.scheme == "http" || request.url.scheme == "https")) {
+ context.startActivity(intent)
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Intent(Intent.ACTION_VIEW, request.url).let { | |
| context.startActivity(it) | |
| } | |
| Intent(Intent.ACTION_VIEW, request.url).let { intent -> | |
| if (intent.resolveActivity(context.packageManager) != null && | |
| (request.url.scheme == "http" || request.url.scheme == "https")) { | |
| context.startActivity(intent) | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/com/sopt/clody/core/security/weview/SecureWebViewClient.kt
around lines 23 to 25, the code launches an intent directly from the URL without
verifying it, which poses a security risk. To fix this, add checks to verify the
intent's scheme and ensure it is safe before calling startActivity. Only allow
trusted schemes like http or https, and prevent launching intents with
potentially dangerous schemes such as intent:// or file://. Implement
conditional logic to validate the URL and safely launch the intent only if it
passes these checks.
| return try { | ||
| Runtime.getRuntime().exec(arrayOf("/system/xbin/which", "su")) | ||
| .inputStream.bufferedReader().readLine() != null | ||
| } catch (e: Exception) { | ||
| false | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential command injection vulnerability and incomplete detection.
The which su command execution has security and reliability issues:
- The command path
/system/xbin/whichmay not exist on all devices - The approach only checks if the command produces output, not if
suactually exists
Consider a safer approach:
- return try {
- Runtime.getRuntime().exec(arrayOf("/system/xbin/which", "su"))
- .inputStream.bufferedReader().readLine() != null
- } catch (e: Exception) {
- false
- }
+ return try {
+ val process = Runtime.getRuntime().exec(arrayOf("which", "su"))
+ val exitCode = process.waitFor()
+ exitCode == 0
+ } catch (e: Exception) {
+ false
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return try { | |
| Runtime.getRuntime().exec(arrayOf("/system/xbin/which", "su")) | |
| .inputStream.bufferedReader().readLine() != null | |
| } catch (e: Exception) { | |
| false | |
| } | |
| return try { | |
| val process = Runtime.getRuntime().exec(arrayOf("which", "su")) | |
| val exitCode = process.waitFor() | |
| exitCode == 0 | |
| } catch (e: Exception) { | |
| false | |
| } |
🧰 Tools
🪛 detekt (1.23.8)
[warning] 43-43: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🤖 Prompt for AI Agents
In
app/src/main/java/com/sopt/clody/core/security/login/DefaultLoginSecurityChecker.kt
around lines 40 to 45, the current code uses a hardcoded path to execute "which
su" that may not exist on all devices and only checks for output presence, which
is unreliable. Replace this with a safer method by searching for "su" in common
system paths programmatically or using a more robust API to verify if "su"
exists and is executable, avoiding direct command execution with fixed paths to
prevent command injection and improve detection accuracy.
2 participants
📌 ISSUE
closed #285
DroidKnights 2025 레드팀 화이트해커의 Offensive 관점을 보고 기립 박수를 치며 가져왔습니다.
📄 Work Description
1. 로그인 보안 점검 로직 추가
LoginSecurityChecker인터페이스 및 구현체DefaultLoginSecurityChecker를 정의하여, 로그인 시 다음과 같은 보안 점검을 사전 수행하도록 했습니다:DefaultLoginSecurityChecker훑어보기isDeviceRooted()루팅 여부는 다음 3단계를 통해 판별합니다:
Build.TAGS에test-keys가 포함되어 있는지 확인su) 및 앱(Superuser.apk)의 존재 여부 확인which su명령어 실행 결과로su의 존재 여부 확인→ 3가지 중 하나라도 해당되면 루팅된 기기로 판단하고 로그인 차단
isChromeInstalled(context)Kakao SDK의 웹 기반 로그인 흐름 보안 강화를 위해 Chrome 브라우저 설치를 필수 조건으로 설정하였습니다.
→
com.android.chrome패키지 존재 여부 확인 후, 미설치 시 로그인 차단3. WebView 보안성 강화
WebView로 통일하고,SecureWebViewClient를 적용하여 다음을 보장:notion.so,forms.gle등)만 로드 가능4. 마켓 이동 시 Intent 보안 강화
AppUpdateUtils내 마켓 이동 로직 개선:com.android.vending) 설정✨ PR Point
리뷰는 안하셔도 됩니다. 오 이런게 있구나 하고 한번 봐주시면 좋을듯~
📸 ScreenShot/Video
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores