fix: 보안 취약점 5건 수정 (Path Traversal, 암호화 키, S3 평문, SQL Injection, 입력 검증)#13
Conversation
- DatasourceService.update()에서도 이름에 경로 문자(/, \, ..) 포함 여부 검증 - LoginRequest.password에 누락된 @field:Size(min=4) 추가
Summary of ChangesHello @clroot, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 코드 보안 리뷰에서 발견된 5가지 주요 보안 취약점(경로 탐색, 기본 암호화 키 사용, S3 자격 증명 평문 저장, SQL Injection, 입력 유효성 검증 부재)을 해결하여 시스템의 전반적인 보안 수준과 견고성을 크게 향상시킵니다. 사용자 입력과 내부 데이터 처리 과정에서 발생할 수 있는 잠재적인 공격 벡터를 차단하고, 민감한 정보의 안전한 저장을 보장함으로써 애플리케이션의 신뢰성을 강화하는 데 중점을 두었습니다. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
| var updated = false | ||
|
|
||
| if (entity.s3AccessKey != null && !entity.s3AccessKey!!.startsWith(ENCRYPTED_PREFIX)) { | ||
| entity.s3AccessKey = ENCRYPTED_PREFIX + encryptionPort.encrypt(entity.s3AccessKey!!) | ||
| updated = true | ||
| } | ||
| if (entity.s3SecretKey != null && !entity.s3SecretKey!!.startsWith(ENCRYPTED_PREFIX)) { | ||
| entity.s3SecretKey = ENCRYPTED_PREFIX + encryptionPort.encrypt(entity.s3SecretKey!!) | ||
| updated = true | ||
| } | ||
|
|
||
| if (updated) repository.save(entity) |
There was a problem hiding this comment.
현재 코드는 s3AccessKey와 s3SecretKey에 대해 거의 동일한 로직을 반복하고 있으며, !! (non-null assertion) 연산자를 사용하고 있습니다.
Kotlin의 let과 takeIf를 사용하여 코드를 더 간결하고 안전하게 개선할 수 있습니다. 이렇게 하면 중복을 줄이고 !! 사용을 피하여 코드 가독성과 안정성을 높일 수 있습니다.
| var updated = false | |
| if (entity.s3AccessKey != null && !entity.s3AccessKey!!.startsWith(ENCRYPTED_PREFIX)) { | |
| entity.s3AccessKey = ENCRYPTED_PREFIX + encryptionPort.encrypt(entity.s3AccessKey!!) | |
| updated = true | |
| } | |
| if (entity.s3SecretKey != null && !entity.s3SecretKey!!.startsWith(ENCRYPTED_PREFIX)) { | |
| entity.s3SecretKey = ENCRYPTED_PREFIX + encryptionPort.encrypt(entity.s3SecretKey!!) | |
| updated = true | |
| } | |
| if (updated) repository.save(entity) | |
| var needsSave = false | |
| entity.s3AccessKey?.takeIf { !it.startsWith(ENCRYPTED_PREFIX) }?.let { | |
| entity.s3AccessKey = ENCRYPTED_PREFIX + encryptionPort.encrypt(it) | |
| needsSave = true | |
| } | |
| entity.s3SecretKey?.takeIf { !it.startsWith(ENCRYPTED_PREFIX) }?.let { | |
| entity.s3SecretKey = ENCRYPTED_PREFIX + encryptionPort.encrypt(it) | |
| needsSave = true | |
| } | |
| if (needsSave) repository.save(entity) |
| val pattern = | ||
| Regex( | ||
| """^\s*"([a-zA-Z_][a-zA-Z0-9_ ]*)"\s*(ASC|DESC)?\s*$|^\s*([a-zA-Z_][a-zA-Z0-9_]*)\s*(ASC|DESC)?\s*$""", | ||
| RegexOption.IGNORE_CASE, | ||
| ) |
There was a problem hiding this comment.
sanitizeOrderBy 함수가 호출될 때마다 Regex 객체를 생성하는 것은 비효율적일 수 있습니다. 이 정규식을 클래스의 companion object에 상수로 추출하여, 클래스 로딩 시 한 번만 컴파일되도록 하는 것이 좋습니다. 이렇게 하면 성능이 개선되고 코드가 더 명확해집니다.
아래와 같이 수정할 수 있습니다:
@Component
class DuckDbQueryEngine : QueryEngine {
companion object {
private val ORDER_BY_PATTERN = Regex(
"""^\s*"([a-zA-Z_][a-zA-Z0-9_ ]*)"\s*(ASC|DESC)?\s*$|^\s*([a-zA-Z_][a-zA-Z0-9_]*)\s*(ASC|DESC)?\s*$""",
RegexOption.IGNORE_CASE,
)
}
// ...
private fun sanitizeOrderBy(orderBy: String): String {
return orderBy.split(",").map { part ->
val match =
ORDER_BY_PATTERN.matchEntire(part.trim())
?: throw IllegalArgumentException("Invalid ORDER BY clause: ${part.trim()}")
// ... (rest of the function)
}.joinToString(", ")
}
}1 participant
Summary
보안 코드리뷰에서 발견된 5건의 보안 취약점을 수정합니다.
LocalStorageAdapter.safePath()경로 경계 검증 +Datasource이름에 경로 문자 금지ENC:접두사 암호화 + 시작 시 기존 평문 자동 마이그레이션@Valid+ 에러 핸들러Changes
Path Traversal 방어
LocalStorageAdapter: 모든 public 메서드에서safePath()경로 경계 검증Datasource.create()/DatasourceService.update(): 이름에/,\,..포함 시 reject기본 암호화 키 제거
AesEncryptionAdapter: dev 프로필에서만 기본 키 허용 (경고 로그), 그 외IllegalStateExceptionS3 자격증명 암호화
StorageConfigMapper: 저장 시ENC:+ 암호화, 읽기 시 자동 복호화 (레거시 평문 호환)StorageConfigMigrationRunner: 시작 시 기존 평문 자격증명 자동 암호화SQL Injection 방어
DuckDbQueryEngine.sanitizeOrderBy():identifier (ASC|DESC)?패턴만 허용, 식별자 이스케이프입력 검증
@RequestBody에@Valid추가GlobalExceptionHandler에MethodArgumentNotValidException핸들러 추가Test plan
LocalStorageAdapterTest— path traversal 차단 (9 cases)DatasourceTest— 이름 경로 문자 검증 (4 cases)DatasourceServiceTest— update 경로 이름 검증AesEncryptionAdapterTest— 프로필별 동작 + 키 길이 (5 cases)StorageConfigMapperTest— 암복호화 + 레거시 호환 (5 cases)StorageConfigMigrationRunnerTest— 마이그레이션 시나리오 (4 cases)DuckDbQueryEngineTest— orderBy 허용/차단 (4 cases)./gradlew test)