백오피스 리뷰 후속 - 보안 5건·동작 3건·정합성 5건 - #989
Open
m-a-king wants to merge 3 commits into
Open
Conversation
- session/extend 의 CSRF 면제를 걷었다. "fetch 라 토큰을 실을 수 없다"는 면제 근거가 틀렸다 - 같은 화면의 다른 fetch 는 이미 헤더로 싣고 있었다. 면제 상태에선 제3자 페이지가 cross-origin 폼 자동 제출로 allowlist TTL 을 무기한 늘려, 연장은 명시적 버튼으로만 한다는 #669 의 보장이 무력화된다. 로컬 실측으로 토큰 없는 POST 가 401 로 막히고 토큰을 실으면 200 인 것을 확인했다 - grant 흐름이 기존 세션에 admin 신원을 심어 세션 픽세이션 창이 열려 있었다. establish 전에 invalidate 후 새 세션을 발급한다. permitAll 체인이라 Spring Security 의 픽세이션 보호가 돌지 않아 IP 바인딩 한 겹에만 기대고 있었다 - 감사 로그 IP 를 XFF 첫 hop 에서 읽던 마지막 한 곳(AdminTemplateController)을 ClientIp.of 로 옮겼다. nginx 가 XFF 를 append 하므로 클라 위조분이 첫 hop 이라 감사 추적이 공격자 통제 아래 있었다 - /admin-assets/** 는 서빙하는 것이 없는데 보안 체인에 permitAll 로 뚫려 있고 게이트 필터도 제외하고 있었다. 정적 핸들러를 붙이는 순간 무인증 공개가 되도록 미리 파인 구멍이라 matcher 를 지웠다 - 변수 칩만 th:utext 로 마크업을 문자열 결합해 조립하고 있었다. 이스케이프되는 th:text 두 개로 나눴다 - 출력은 같고 unescaped sink 만 사라진다. 변수 카탈로그가 DB 로 옮겨가면 그대로 저장형 XSS 경로가 된다 - fragments 의 CSRF 값은 safe navigation 으로 읽는다. 직접 접근하면 CsrfFilter 가 없는 구성에서 상단바 렌더가 깨져 admin 페이지 전체가 500 이 된다(테스트에서 실제로 터졌다)
- 공지 send·cancel·delete 가 IllegalArgumentException 을 안 잡아, 발송 버튼 더블클릭 같은 흔한 레이스에서 @RestControllerAdvice 가 SSR 페이지를 raw JSON 400 으로 갈아치웠다. 운영자는 화면을 잃고 발송이 시작됐는지도 알 수 없었다. 형제 컨트롤러들이 이미 쓰는 try/catch 패턴을 적용하고 사유별 배너를 붙였다 - 정책·출처 몰 상세의 catch 블록이 복구용으로 부르는 find 가 다시 던질 수 있었다. 복구 경로에서 예외가 새면 에러 메시지와 제출값을 함께 잃는다. runCatching 으로 감싸 그 경우 목록으로 돌린다 - 발송 결과 폴링에 상한과 중단 조건이 없어, 삭제된 공지나 SENDING 에 갇힌 공지에서 탭 하나가 시간당 약 1200 요청을 영구히 보냈다. 비 2xx 는 영구 오류로 보고 즉시 멈추고, 진행이 안 끝나도 약 10분에서 멈춰 새로고침을 안내한다 - 배너를 붙이다 param.error 비교가 원래부터 죽어 있던 것을 발견했다. param.error 는 String[] 이라 문자열과 직접 비교하면 항상 false 다(metrics.html 만 param.error[0] 로 올바르게 쓰고 있었다). 새로 넣은 3건과 기존 length·image 2건을 함께 고쳤고, 로컬에서 다섯 배너가 모두 뜨는 것을 확인했다
- flatpickr 2건이 무버전 URL 이라 jsDelivr 의 latest 로 해석되고 있었다. breaking 릴리즈가 나면 dateFormat 계약이 조용히 바뀌어 예약 시각이 서버가 못 읽는 모양으로 제출된다. 4.6.13 으로 고정하고, 무버전 URL 이 해석하던 파일과 내용이 같음을 해시로 확인했다 - pretendard·flatpickr 자원에 SRI(integrity·crossorigin)를 걸었다. 이 화면들은 CSRF 토큰을 JS 변수로 들고 전체 사용자 푸시 브로드캐스트를 구동하므로, 무결성 검증 없는 스크립트가 admin 세션 범위에서 도는 상태였다 - toast editor 4건은 SRI 를 걸지 못했다. uicdn.toast.com 이 CORS 헤더를 주지 않아 crossorigin 을 붙이는 순간 브라우저가 차단한다 - 실제로 붙였다가 공지 편집기가 통째로 안 뜨는 것을 로컬에서 확인하고 되돌렸다. CORS 를 주는 jsDelivr 로 옮기면 걸 수 있으나 같은 3.2.2 라도 빌드가 달라(해시 불일치) 별도 확인이 필요해 TODO 로 남겼다 - admin 빈 4개에 @ConditionalOnAdminEnabled 가 빠져 있었다. admin.enabled=false 에서도 생성되고, 그중 DefaultPushImage 는 생성 시점에 S3 프로퍼티를 읽어 부팅을 깬다. 그 어노테이션의 KDoc 이 막으려던 바로 그 누락이다 - EnvironmentAccessFilter 의 @order 가 AccessLogFilter 와 같은 값이라 체인 내 위치가 미정이었다. 게이트가 바깥으로 정렬되면 dev 문서 탐지 시도가 access log 도 traceId 도 없이 사라진다. 한 칸 안으로 내려 순서를 확정했다 - StatsEmbed 가 같은 패키지 DiscordInteractions 의 상수를 private 으로 재선언하고 있었다. 값이 갈리면 /stats 만 공개로 올라가는 식으로 조용히 어긋나므로 참조로 바꿨다 - /docs grant 안내가 IP 등록 수명을 1시간이라 알렸으나 실제 TTL 은 24시간이다. 문구를 설정값에서 파생하게 바꿔 다시 어긋나지 않게 했다
|
Discord 스레드 연동용 메타데이터입니다. discord-pr-bot 워크플로가 자동 생성하며, 수정·삭제하면 PR 과 Discord 알림 연동이 끊깁니다. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: 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. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Situation
백오피스 전체(admin 패키지 47개 파일 + 템플릿 16개)를 코드 리뷰해 15건이 나왔다. 가장 심각한 게이트 우회는 #987 에서 먼저 냈고, 이 PR 이 나머지다.
절반 가까이가 코드베이스가 이미 산문으로 정해둔 규칙을 딱 한 곳만 안 지킨 경우였다.
ClientIp.of쓰기(19곳 중 1곳 누락), admin 빈에@ConditionalOnAdminEnabled달기(4개 누락), SSR 컨트롤러의 try/catch, 공통 응답 래퍼. 규칙과 참조 구현이 이미 있어 고치는 비용이 낮았다.Task
14건을 문제별로 닫는다. 각 수정이 의도대로 동작하는지, 그리고 고치려던 문제가 실제로 존재했는지를 로컬 실행으로 확인한다.
Action
보안 5건
POST /admin/session/extend가 CSRF 면제establish전에invalidate()후 새 세션 발급ClientIp.of로 교체 (19곳 중 유일하게 남아 있던 한 곳)/admin-assets/**가 permitAll 로 뚫려 있음th:utext로 마크업 조립th:text두 개로 분리 (출력은 동일)동작 4건
send·cancel·delete가 예외를 안 잡음find가 재throwrunCatching으로 감싸고, 실패하면 목록으로 되돌린다param.error비교가 항상 falseString[]을 문자열과 직접 비교해, 길이 초과·이미지 실패 배너가 원래부터 한 번도 뜬 적이 없다param.error[0]로 수정. 새로 넣은 3건과 기존 2건을 함께 고쳤다마지막 항목은 리뷰에 없던 건이다. 배너를 붙이다 기존 것도 죽어 있는 걸 발견했다 (
metrics.html만 인덱스 접근으로 올바르게 쓰고 있었다).정합성 5건
dateFormat계약이 조용히 바뀌어 예약 시각이 서버가 못 읽는 모양으로 제출된다integrity·crossorigin적용 (toast editor 는 아래 참조)@ConditionalOnAdminEnabled누락admin.enabled=false에서도 생성된다.DefaultPushImage는 생성 시점에 S3 프로퍼티를 읽어 부팅을 깬다EnvironmentAccessFilter의@Order가AccessLogFilter와 동일/docsTTL 문구 오기/stats만 공개로 올라가거나, 응답 모양을 통일하려는 다음 사람이 카운트다운을 깬다. TTL 은 실제 24시간인데 1시간이라 안내해 아무도 회수하지 않는다ApiResponseBody로, TTL 문구는 설정값에서 파생하게Result
로컬에 앱을 띄워 각 수정을 실행으로 확인했다.
POST /admin/session/extend{"data":{"remainingSeconds":0},...}data-var유지못 한 것 하나: toast editor 의 SRI
uicdn.toast.com이 CORS 헤더를 주지 않는다. SRI 는 교차 출처에서 CORS 없이 동작하지 않으므로,crossorigin을 붙이는 순간 브라우저가 스크립트를 차단한다. 실제로 붙였다가 공지 편집기가 통째로 안 뜨는 것(window.toastuiundefined, 에디터 미마운트)을 로컬에서 확인하고 되돌렸다.@toast-ui/editor@3.2.2로 이전이 PR 은 현행 유지로 두고 코드에 TODO 를 남겼다. 어느 쪽으로 갈지는 별도로 정한다.
연관 이슈