Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ public enum Whitelist {
SWAGGER_UI("/api/v1/swagger-ui", WhitelistMethod.ALL),
VIRTUAL_ACCOUNT("/api/v1/virtual-account", WhitelistMethod.ALL),
ADMISSION_TICKET("/api/v1/admission-ticket", WhitelistMethod.ALL),
FILE("/api/v1/s3", WhitelistMethod.ALL),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

Whitelisting the S3 upload endpoint /api/v1/s3 allows requests to bypass token-based authentication filters. In combination with the removal of @PreAuthorize in FileController, this makes the file upload functionality completely public.

This poses a critical security risk. Please see my comment on FileController.java for a detailed explanation of the vulnerabilities and recommended actions. This line should be removed to ensure all API requests go through the standard authentication and authorization process.


// 정적 리소스
CSS("/api/v1/css", WhitelistMethod.GET),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import life.mosu.mosuserver.presentation.file.dto.FileUploadResponse;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpStatus;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
Expand All @@ -23,7 +22,6 @@ public class FileController {
private final S3Service s3Service;

@PostMapping

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

Removing the @PreAuthorize annotation makes this file upload endpoint public and unauthenticated, introducing a critical security vulnerability. I've suggested re-adding it to secure the endpoint.

An unauthenticated user can now perform the following malicious actions:

  • Denial of Service (DoS): Upload an unlimited number of files to your S3 bucket, which can lead to significant financial costs and service disruption.
  • Malicious File Upload: Upload harmful content like malware, viruses, or phishing pages.
  • Arbitrary Folder Upload: The folderName request parameter allows an attacker to choose the destination folder. Based on the Folder enum, they can upload to PUBLIC folders like EVENT, FAQ, and NOTICE. This would allow them to host malicious content that appears to be from your service, as it would be served from your S3 bucket.
Suggested change
@PostMapping
@PreAuthorize("isAuthenticated() and hasRole('USER')")

@PreAuthorize("isAuthenticated() and hasRole('USER')")
public ApiResponseWrapper<FileUploadResponse> upload(
@RequestParam("file") MultipartFile file,
@RequestParam(defaultValue = "temp") String folderName
Expand Down