Conversation
…e PR #52) develop <- feat/web/dynamic-view#23
- 필터 단에서 redirect를 쉽게 설정하기 위해 redirectTo 메서드 추가
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
전체 리뷰 의견
로그아웃 핸들러 구현은 기본적인 기능은 잘 정리되어 있습니다. 다만 요청하신 취약점 검토 항목으로 3개의 보안 문제를 발견했습니다:
-
CSRF 취약점 (Critical): 로그아웃 폼과 핸들러에 CSRF 토큰 검증이 없어 악의적 웹사이트에서 사용자를 공격할 수 있습니다.
-
오픈 리다이렉트 취약점 (High):
redirectTo()메서드가 입력값을 검증하지 않아, 제어되지 않은 경로가 전달되면 외부 사이트로 리다이렉트될 수 있습니다. -
세션 무효화 에러 핸들링 부재 (Medium): 세션 무효화 실패 여부를 확인하지 않아 예상치 못한 동작이 발생할 수 있습니다.
코드 리팩토링(변수명 변경)과 HttpResponse.redirectTo() 유틸리티 메서드 추가는 좋은 개선입니다.
| public HandlerResponse handle(HttpRequest request) { | ||
| String sid = request.getCookieValue("SID").orElse(null); |
There was a problem hiding this comment.
잠재적 동시성 문제: getCookieValue() 호출 후에 sessionManager.invalidate()를 호출하는데, 이 사이에 다른 요청이 같은 세션 ID로 진입할 수 있습니다. 또한 로그아웃 처리 후 쿠키 삭제까지의 과정에서 TOCTOU(Time-of-check-time-of-use) 문제가 존재할 수 있습니다. 세션 무효화 실패 여부를 확인하고 에러 핸들링을 추가하는 것이 좋습니다.
| public HandlerResponse handle(HttpRequest request) { | ||
| String sid = request.getCookieValue("SID").orElse(null); | ||
| if (sid != null) sessionManager.invalidate(sid); |
There was a problem hiding this comment.
CSRF 취약점: 로그아웃 폼이 CSRF 토큰 없이 POST 요청을 수락하고 있습니다. 헤더.html 의 폼에서 CSRF 토큰을 포함시키고, 핸들러에서 이를 검증해야 합니다.
| <form action="/user/logout" method="POST"> | ||
| <button type="submit">로그아웃</button> | ||
| </form> |
There was a problem hiding this comment.
CSRF 토큰 누락: 로그아웃 폼에서 CSRF 방지 토큰이 없습니다. 악의적 웹사이트에서 사용자를 공격할 수 있습니다. 숨겨진 입력 필드로 CSRF 토큰을 추가하세요.
| public void redirectTo(String path){ | ||
| setStatus(HttpStatus.FOUND); | ||
| setHeader("Location", path); | ||
| setHeader("Content-Length", "0"); | ||
| } |
There was a problem hiding this comment.
경로 검증 누락: redirectTo() 메서드가 경로 입력값을 검증하지 않습니다. 이 메서드를 사용하는 곳에서 값이 제어되지 않은 입력을 받으면 오픈 리다이렉트 취약점이 발생할 수 있습니다. 경로가 상대 경로(또는 같은 호스트)인지 검증하거나, URL 파싱을 통해 악의적 리다이렉트를 방지하세요.
1 participant
💻 작업 내용
✨ 리뷰 포인트
특별할 것 없는 로그아웃 핸들러 개발입니다.
제가 놓친 취약점이 있는지 체크해주세요.
🎯 관련 이슈
closed #51