diff --git a/.jules/sentinel.md b/.jules/sentinel.md index e795cb9..85ad11d 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -32,3 +32,8 @@ **Vulnerability:** The document hashing routine in `DefaultDocumentConversionService` processed file streams without enforcing any maximum size limit on the bytes read. An attacker could exploit this by uploading a maliciously large stream (or exploiting a compression bomb if unzipping), exhausting system memory, CPU, or disk space (DoS). **Learning:** Checking the declared file size (e.g., `file.getSize()`) in initial validation is not always sufficient if the input stream itself can be spoofed or dynamically expanded during reading. The actual bytes read must be verified against bounds continuously. **Prevention:** Always enforce a strict, configurable size limit (e.g., `ConversionProperties.maxUploadSizeBytes`) within the `while` loop that reads from untrusted input streams. Track `totalRead` and throw an exception immediately if the limit is exceeded. + +## 2026-07-15 - 정책 오버라이드 승인자 ID PII 로깅 취약점 +**Vulnerability:** 차단된 형식의 정책 오버라이드가 수락되었을 때, 문서 검증 서비스가 평문 형태의 `approverId`를 로깅하고 있었습니다. 이 값은 민감한 식별 정보(PII)를 포함할 수 있으므로 이를 평문으로 기록하는 것은 PII 로깅 정책 위반입니다. +**Learning:** 로그 삽입 등을 방지하기 위해 보안 토큰이나 식별자를 정리(sanitize)하더라도, 해당 값이 PII로 간주되는 경우 민감한 데이터가 중앙 로깅 시스템에 유출되는 것을 막기 위해 반드시 해시 처리 또는 지문화(fingerprinting)를 적용해야 합니다. +**Prevention:** PII 로깅 정책을 준수하려면 정책 오버라이드의 `approverId`와 같은 민감한 식별자를 평문으로 로깅해서는 안 됩니다. 감사 로그를 기록하기 전에 반드시 널(null) 안정성을 포함한 해싱 또는 지문화(예: SHA-256 해시 및 16진수 인코딩)를 적용해야 합니다. diff --git a/commit_message.txt b/commit_message.txt index 32f3527..47d5b89 100644 --- a/commit_message.txt +++ b/commit_message.txt @@ -1,4 +1,4 @@ -🛡️ Sentinel: [CRITICAL] 파일 업로드 경로 조작(Path Traversal) 취약점 수정 +🛡️ Sentinel: [CRITICAL] PII 로그 유출 취약점 수정 (approverId 해싱) -MultipartFile.getOriginalFilename()을 신뢰하여 발생할 수 있는 경로 조작 취약점을 수정했습니다. -StringUtils.cleanPath()를 사용하여 경로를 정규화하고 순수한 파일명만 추출하여 악의적인 페이로드(예: ../../../etc/passwd.hwp)로부터 시스템을 보호합니다. +문서 검증 서비스에서 정책 오버라이드 수락 시 `approverId`가 평문으로 로그에 기록되어 PII(개인식별정보)가 유출될 위험이 있었습니다. +`approverId`를 기록하기 전에 SHA-256 해싱 및 16진수 인코딩을 수행하는 `fingerprintApproverId` 메서드를 추가하여 이를 해결했습니다. diff --git a/description.txt b/description.txt index 861a67d..1255d80 100644 --- a/description.txt +++ b/description.txt @@ -1,5 +1,5 @@ 🚨 Severity: CRITICAL -💡 Vulnerability: 파일 업로드 시 `MultipartFile.getOriginalFilename()` 값을 검증 없이 사용하여 발생할 수 있는 경로 조작(Path Traversal) 취약점 발견. -🎯 Impact: 공격자가 디렉토리 탐색 문자열(`../`)을 포함한 파일명을 전송하여 의도하지 않은 경로에 파일을 저장하거나 시스템 파일(예: `/etc/passwd`)에 접근/조작할 위험이 있음. -🔧 Fix: `DefaultDocumentConversionService` 및 `DefaultDocumentValidationService`에서 파일명을 사용하기 전 `StringUtils.cleanPath()`를 통해 경로를 정규화하고, 마지막 `/` 이후의 순수한 파일명만 추출하도록 `sanitizeFilename` 메소드를 추가하여 안전하게 처리함. -✅ Verification: 단위 테스트(`submitStripsDirectoryTraversalFromOriginalFilename` 및 `stripsDirectoryTraversalFromFilename` 등)를 추가하여 취약점 문자열이 정상적으로 제거되며 100% 테스트 커버리지를 보장함. +💡 Vulnerability: 문서 검증 서비스에서 정책 오버라이드 수락 시 `approverId`가 평문으로 로그에 기록되어 PII(개인식별정보)가 유출될 위험이 있었습니다. +🎯 Impact: 공격자나 내부 직원이 로그 시스템을 통해 민감한 식별 정보를 열람하여 개인정보 침해나 권한 도용으로 이어질 수 있습니다. +🔧 Fix: `approverId`를 기록하기 전에 널(null) 안정성이 확보된 SHA-256 해싱 및 16진수 인코딩을 수행하는 `fingerprintApproverId` 메서드를 추가하여 안전하게 지문화(fingerprinting)하도록 수정했습니다. 성능 최적화 가이드라인에 따라 기존의 단일 `HEX_FORMAT` 상수를 재사용했습니다. 테스트 코드도 100% 커버리지를 만족하도록 추가하였습니다. +✅ Verification: `mvn test`를 실행하여 새로 추가된 `fingerprintApproverIdReturnsEmptyWhenInputIsNull` 및 `throwsWhenSha256DigestIsUnavailableForApproverIdFingerprint` 테스트를 통과하는지 확인했습니다. diff --git a/src/main/java/com/clearfolio/viewer/service/DefaultDocumentValidationService.java b/src/main/java/com/clearfolio/viewer/service/DefaultDocumentValidationService.java index 95e6722..8b36afb 100644 --- a/src/main/java/com/clearfolio/viewer/service/DefaultDocumentValidationService.java +++ b/src/main/java/com/clearfolio/viewer/service/DefaultDocumentValidationService.java @@ -115,7 +115,7 @@ public void validateOrThrow(MultipartFile file, PolicyOverrideRequest overrideRe LOGGER.info( "Blocked-format override accepted extension={} approverId={} tokenFingerprint={}", sanitizeForLog(extension), - sanitizeForLog(overrideApproverIdForAudit), + fingerprintApproverId(overrideApproverIdForAudit), tokenFingerprint(overrideTokenForAudit) ); } @@ -209,6 +209,20 @@ private String tokenFingerprint(String approvalToken) { } } + private String fingerprintApproverId(String approverId) { + if (approverId == null || approverId.isBlank()) { + return ""; + } + try { + MessageDigest digest = MessageDigest.getInstance("SHA-256"); + byte[] hashed = digest.digest(approverId.getBytes(StandardCharsets.UTF_8)); + // Reused HexFormat for performance + return HEX_FORMAT.formatHex(hashed); + } catch (NoSuchAlgorithmException ex) { + throw new IllegalStateException("SHA-256 digest unavailable", ex); + } + } + private String sanitizeForLog(final String value) { if (value == null) { return ""; diff --git a/src/test/java/com/clearfolio/viewer/service/DefaultDocumentValidationServiceTest.java b/src/test/java/com/clearfolio/viewer/service/DefaultDocumentValidationServiceTest.java index 4f27bdc..ea2c64d 100644 --- a/src/test/java/com/clearfolio/viewer/service/DefaultDocumentValidationServiceTest.java +++ b/src/test/java/com/clearfolio/viewer/service/DefaultDocumentValidationServiceTest.java @@ -525,6 +525,46 @@ void sanitizeForLogReplacesTabCharacter() throws Exception { assertEquals("approver_id", sanitized); } + @Test + void fingerprintApproverIdReturnsEmptyWhenInputIsNull() throws Exception { + ConversionProperties conversionProperties = new ConversionProperties(); + DefaultDocumentValidationService validationService = new DefaultDocumentValidationService(conversionProperties); + Method method = DefaultDocumentValidationService.class.getDeclaredMethod("fingerprintApproverId", String.class); + method.setAccessible(true); + + String result = (String) method.invoke(validationService, new Object[] {null}); + + assertEquals("", result); + } + + @Test + void throwsWhenSha256DigestIsUnavailableForApproverIdFingerprint() throws Exception { + ConversionProperties conversionProperties = new ConversionProperties(); + DefaultDocumentValidationService validationService = new DefaultDocumentValidationService(conversionProperties); + Method method = DefaultDocumentValidationService.class.getDeclaredMethod("fingerprintApproverId", String.class); + method.setAccessible(true); + + synchronized (SECURITY_PROVIDERS_LOCK) { + Provider[] providers = Security.getProviders(); + for (Provider provider : providers) { + Security.removeProvider(provider.getName()); + } + + try { + java.lang.reflect.InvocationTargetException ex = assertThrows( + java.lang.reflect.InvocationTargetException.class, + () -> method.invoke(validationService, "approver-1") + ); + + assertEquals("SHA-256 digest unavailable", ex.getCause().getMessage()); + } finally { + for (int index = 0; index < providers.length; index++) { + Security.insertProviderAt(providers[index], index + 1); + } + } + } + } + @Test void throwsWhenSha256DigestIsUnavailableForOverrideAuditFingerprint() { ConversionProperties conversionProperties = new ConversionProperties();