[Web-Argument] Multipart 타입 Argument 개발#60
Conversation
develop <- enhance/view/#56
- Multipart request의 body 중 파일 타입을 담는 MultipartFile 정의 - Multipart request의 body 중 파일이 아닌 form data와 MultipartFile을 저장하는 MultipartForm 클래스 개발
- MultipartRequest에 대해 MultipartForm을 생성하는 파서 개발 - 파서 테스트 개발
- MultipartFormParser를 사용해 MultipartForm 타입 argument를 생성하는 ArgumentResolver 개발 - argument resolver에 대한 테스트 개발
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| if (contentTypeHeader == null || !contentTypeHeader.toLowerCase().startsWith("multipart/form-data")) { | ||
| throw new ServiceException(ErrorCode.INVALID_INPUT, "Content-Type must be multipart/form-data"); | ||
| } | ||
|
|
||
| String boundary = extractBoundary(contentTypeHeader); | ||
| if (boundary == null || boundary.isBlank()) { | ||
| throw new ServiceException(ErrorCode.INVALID_INPUT, "boundary missing in Content-Type"); |
There was a problem hiding this comment.
메모리 오버플로우 위험: 바운더리가 매우 길거나 반복되는 패턴으로 구성되면 indexOf() 호출에서 시간 복잡도가 O(n*m)이 되어 ReDoS 같은 성능 공격에 취약합니다. 바운더리 길이를 제한(예: 최대 256바이트)하거나 더 효율적인 문자열 검색 알고리즘 사용을 권장합니다."
| import java.nio.charset.StandardCharsets; | ||
| import java.util.*; | ||
|
|
||
| public class MultipartFormParser { |
There was a problem hiding this comment.
전체 바디 크기 제한 없음: 매우 큰 multipart 바디(예: 기가바이트)가 메모리에 모두 로드될 수 있어 OOM 위험이 있습니다. 최대 크기 제한을 추가하고, 초과하면 ServiceException을 던져야 합니다."
| // 1) headers | ||
| Map<String, String> headers = new LinkedHashMap<>(); | ||
| while (true) { | ||
| int lineEnd = indexOf(body, "\r\n".getBytes(StandardCharsets.ISO_8859_1), pos); | ||
| if (lineEnd < 0) throw new ServiceException(ErrorCode.INVALID_INPUT, "Invalid multipart headers"); | ||
|
|
||
| if (lineEnd == pos) { | ||
| pos += 2; |
There was a problem hiding this comment.
무한 루프 위험: indexOf()가 -1을 반환하면 루프가 종료되지만, 잘못된 바운더리로 데이터가 손상된 multipart 바디가 들어올 경우 상태 추적이 복잡해져서 로직 오류 가능성이 있습니다. 파싱 상태 기계를 더 엄격하게 검증하고, 무한 루프 방지를 위해 최대 파트 개수 제한을 추가하는 것을 권장합니다."
| files.computeIfAbsent(parsed.name, k -> new ArrayList<>()).add(mf); | ||
| } else { | ||
| String value = new String(partBody, StandardCharsets.UTF_8); |
There was a problem hiding this comment.
파일 메타데이터 무시: 클라이언트에서 보낸 filename 파라미터를 파일 저장에 사용하지 않고 무시하는 것은 좋은 설계입니다. 하지만 향후 파일 저장 시 경로 조작(path traversal) 공격을 방지하기 위해 filename 자체를 검증하거나, 완전히 서버에서 생성된 이름만 사용하도록 강제해야 합니다."
| return b; | ||
| } | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| private int skipCrlf(byte[] body, int pos) { | ||
| if (pos + 2 <= body.length | ||
| && body[pos] == '\r' | ||
| && body[pos + 1] == '\n') { | ||
| return pos + 2; |
There was a problem hiding this comment.
바운더리 검증 부족: extractBoundary() 메서드가 경계값을 검증하지 않습니다. 예를 들어, 빈 바운더리나 특수 문자만 포함된 바운더리는 파싱을 복잡하게 만들거나 예측 불가능한 동작을 초래할 수 있습니다. 바운더리가 printable ASCII 범위에 있고 길이가 합리적인지 검증해야 합니다."
| int eq = t.indexOf('='); | ||
| if (eq < 0) { | ||
| continue; | ||
| } | ||
| String k = t.substring(0, eq).trim().toLowerCase(); | ||
| String v = t.substring(eq + 1).trim(); | ||
|
|
||
| if (v.startsWith("\"") && v.endsWith("\"") && v.length() >= 2) { | ||
| v = v.substring(1, v.length() - 1); | ||
| } | ||
|
|
There was a problem hiding this comment.
따옴표 이스케이프 미처리: ContentDisposition.parse()에서 파라미터 값의 따옴표만 제거하고, 그 안의 이스케이프 시퀀스(예: \\\"나 \\\\)는 처리하지 않습니다. RFC 7578에 따르면 이스케이프된 문자를 올바르게 디코딩해야 하며, 그렇지 않으면 특정 입력 패턴에서 예상 밖의 동작이 발생할 수 있습니다."
|
|
||
| public MultipartFile(String fieldName, String contentType, byte[] bytes) { | ||
| if (fieldName == null || fieldName.isBlank()) { | ||
| throw new ErrorException("MultipartFile: fieldName required"); | ||
| } | ||
| this.fieldName = fieldName; | ||
| this.contentType = (contentType == null || contentType.isBlank()) ? null : contentType; | ||
| this.bytes = Objects.requireNonNullElseGet(bytes, () -> new byte[0]); |
There was a problem hiding this comment.
레코드의 immutability 우회 위험: byte[]는 mutable입니다. 외부에서 multipartFile.bytes() 호출 후 반환된 배열을 직접 수정하면 MultipartFile 인스턴스의 데이터가 변조될 수 있습니다. 방어적 복사(new byte[bytes.length]; System.arraycopy(...))를 추가하거나, bytes() 메서드에서 복사본을 반환해야 합니다."
develop <- feat/app/image/#59
1 participant
💻 작업 내용
✨ 리뷰 포인트
파서쪽에 문제될만한 부분이 있는지 취약점을 체크해주세요.
🎯 관련 이슈
closed #59