From 958ec82ec172dc0efdc87748df4ac078305ca2f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joe=20K=C3=BCng?= Date: Tue, 3 Mar 2026 13:09:55 +0100 Subject: [PATCH] fix(deploy): fix security --- .../admin/AdminOrderController.java | 32 +++++++- .../service/FileSystemStorageService.java | 37 +++++++--- .../service/SlicerService.java | 73 ++++++++++++++----- 3 files changed, 110 insertions(+), 32 deletions(-) diff --git a/backend/src/main/java/com/printcalculator/controller/admin/AdminOrderController.java b/backend/src/main/java/com/printcalculator/controller/admin/AdminOrderController.java index b8d3c46..0fa09b3 100644 --- a/backend/src/main/java/com/printcalculator/controller/admin/AdminOrderController.java +++ b/backend/src/main/java/com/printcalculator/controller/admin/AdminOrderController.java @@ -30,6 +30,8 @@ import org.springframework.web.bind.annotation.RestController; import org.springframework.web.server.ResponseStatusException; import java.nio.charset.StandardCharsets; +import java.nio.file.InvalidPathException; +import java.nio.file.Path; import java.nio.file.Paths; import java.util.List; import java.util.Locale; @@ -144,9 +146,13 @@ public class AdminOrderController { if (relativePath == null || relativePath.isBlank() || "PENDING".equals(relativePath)) { throw new ResponseStatusException(NOT_FOUND, "File not available"); } + Path safeRelativePath = resolveOrderItemRelativePath(relativePath, orderId, orderItemId); + if (safeRelativePath == null) { + throw new ResponseStatusException(NOT_FOUND, "File not available"); + } try { - Resource resource = storageService.loadAsResource(Paths.get(relativePath)); + Resource resource = storageService.loadAsResource(safeRelativePath); MediaType contentType = MediaType.APPLICATION_OCTET_STREAM; if (item.getMimeType() != null && !item.getMimeType().isBlank()) { try { @@ -276,9 +282,9 @@ public class AdminOrderController { private ResponseEntity generateDocument(Order order, boolean isConfirmation) { String displayOrderNumber = getDisplayOrderNumber(order); if (isConfirmation) { - String relativePath = "orders/" + order.getId() + "/documents/confirmation-" + displayOrderNumber + ".pdf"; + Path relativePath = buildConfirmationPdfRelativePath(order.getId(), displayOrderNumber); try { - byte[] existingPdf = storageService.loadAsResource(Paths.get(relativePath)).getInputStream().readAllBytes(); + byte[] existingPdf = storageService.loadAsResource(relativePath).getInputStream().readAllBytes(); return ResponseEntity.ok() .header(HttpHeaders.CONTENT_DISPOSITION, "attachment; filename=\"confirmation-" + displayOrderNumber + ".pdf\"") .contentType(MediaType.APPLICATION_PDF) @@ -298,4 +304,24 @@ public class AdminOrderController { .contentType(MediaType.APPLICATION_PDF) .body(pdf); } + + private Path resolveOrderItemRelativePath(String storedRelativePath, UUID orderId, UUID orderItemId) { + try { + Path candidate = Path.of(storedRelativePath).normalize(); + if (candidate.isAbsolute()) { + return null; + } + Path expectedPrefix = Path.of("orders", orderId.toString(), "3d-files", orderItemId.toString()); + if (!candidate.startsWith(expectedPrefix)) { + return null; + } + return candidate; + } catch (InvalidPathException e) { + return null; + } + } + + private Path buildConfirmationPdfRelativePath(UUID orderId, String orderNumber) { + return Path.of("orders", orderId.toString(), "documents", "confirmation-" + orderNumber + ".pdf"); + } } diff --git a/backend/src/main/java/com/printcalculator/service/FileSystemStorageService.java b/backend/src/main/java/com/printcalculator/service/FileSystemStorageService.java index 7db4aa8..38e1e25 100644 --- a/backend/src/main/java/com/printcalculator/service/FileSystemStorageService.java +++ b/backend/src/main/java/com/printcalculator/service/FileSystemStorageService.java @@ -21,10 +21,12 @@ import java.nio.file.StandardCopyOption; public class FileSystemStorageService implements StorageService { private final Path rootLocation; + private final Path normalizedRootLocation; private final ClamAVService clamAVService; public FileSystemStorageService(@Value("${storage.location:storage_orders}") String storageLocation, ClamAVService clamAVService) { this.rootLocation = Paths.get(storageLocation); + this.normalizedRootLocation = this.rootLocation.toAbsolutePath().normalize(); this.clamAVService = clamAVService; } @@ -39,10 +41,7 @@ public class FileSystemStorageService implements StorageService { @Override public void store(MultipartFile file, Path destinationRelativePath) throws IOException { - Path destinationFile = this.rootLocation.resolve(destinationRelativePath).normalize().toAbsolutePath(); - if (!destinationFile.getParent().startsWith(this.rootLocation.toAbsolutePath())) { - throw new StorageException("Cannot store file outside current directory."); - } + Path destinationFile = resolveInsideStorage(destinationRelativePath); // 1. Salva prima il file su disco per evitare problemi di stream con file grandi Files.createDirectories(destinationFile.getParent()); @@ -63,32 +62,46 @@ public class FileSystemStorageService implements StorageService { @Override public void store(Path source, Path destinationRelativePath) throws IOException { - Path destinationFile = this.rootLocation.resolve(destinationRelativePath).normalize().toAbsolutePath(); - if (!destinationFile.getParent().startsWith(this.rootLocation.toAbsolutePath())) { - throw new StorageException("Cannot store file outside current directory."); - } + Path destinationFile = resolveInsideStorage(destinationRelativePath); Files.createDirectories(destinationFile.getParent()); Files.copy(source, destinationFile, StandardCopyOption.REPLACE_EXISTING); } @Override public void delete(Path path) throws IOException { - Path file = rootLocation.resolve(path); + Path file = resolveInsideStorage(path); Files.deleteIfExists(file); } @Override public Resource loadAsResource(Path path) throws IOException { try { - Path file = rootLocation.resolve(path); + Path file = resolveInsideStorage(path); Resource resource = new UrlResource(file.toUri()); if (resource.exists() || resource.isReadable()) { return resource; } else { - throw new RuntimeException("Could not read file: " + path); + throw new StorageException("Could not read file: " + path); } } catch (MalformedURLException e) { - throw new RuntimeException("Could not read file: " + path, e); + throw new StorageException("Could not read file: " + path, e); } } + + private Path resolveInsideStorage(Path relativePath) { + if (relativePath == null) { + throw new StorageException("Path is required."); + } + + Path normalizedRelative = relativePath.normalize(); + if (normalizedRelative.isAbsolute()) { + throw new StorageException("Cannot access absolute paths."); + } + + Path resolved = normalizedRootLocation.resolve(normalizedRelative).normalize(); + if (!resolved.startsWith(normalizedRootLocation)) { + throw new StorageException("Cannot access files outside storage root."); + } + return resolved; + } } diff --git a/backend/src/main/java/com/printcalculator/service/SlicerService.java b/backend/src/main/java/com/printcalculator/service/SlicerService.java index f9ef570..da782b4 100644 --- a/backend/src/main/java/com/printcalculator/service/SlicerService.java +++ b/backend/src/main/java/com/printcalculator/service/SlicerService.java @@ -10,9 +10,9 @@ import org.springframework.stereotype.Service; import java.io.File; import java.io.IOException; import java.nio.file.Files; +import java.nio.file.InvalidPathException; import java.nio.file.Path; import java.nio.charset.StandardCharsets; -import java.util.ArrayList; import java.util.Comparator; import java.util.List; import java.util.Map; @@ -30,7 +30,7 @@ public class SlicerService { private static final Pattern SIZE_Y_PATTERN = Pattern.compile("(?m)^\\s*size_y\\s*=\\s*([-+]?\\d+(?:\\.\\d+)?)\\s*$"); private static final Pattern SIZE_Z_PATTERN = Pattern.compile("(?m)^\\s*size_z\\s*=\\s*([-+]?\\d+(?:\\.\\d+)?)\\s*$"); - private final String slicerPath; + private final String trustedSlicerPath; private final ProfileManager profileManager; private final GCodeParser gCodeParser; private final ObjectMapper mapper; @@ -40,7 +40,7 @@ public class SlicerService { ProfileManager profileManager, GCodeParser gCodeParser, ObjectMapper mapper) { - this.slicerPath = slicerPath; + this.trustedSlicerPath = normalizeExecutablePath(slicerPath); this.profileManager = profileManager; this.gCodeParser = gCodeParser; this.mapper = mapper; @@ -83,17 +83,23 @@ public class SlicerService { basename = basename.substring(0, basename.length() - 4); } Path slicerLogPath = tempDir.resolve("orcaslicer.log"); + String machineProfilePath = requireSafeArgument(mFile.getAbsolutePath(), "machine profile path"); + String processProfilePath = requireSafeArgument(pFile.getAbsolutePath(), "process profile path"); + String filamentProfilePath = requireSafeArgument(fFile.getAbsolutePath(), "filament profile path"); + String outputDirPath = requireSafeArgument(tempDir.toAbsolutePath().toString(), "output directory path"); + String inputStlPath = requireSafeArgument(inputStl.getAbsolutePath(), "input STL path"); // 3. Run slicer. Retry with arrange only for out-of-volume style failures. for (boolean useArrange : new boolean[]{false, true}) { - List command = new ArrayList<>(); - command.add(slicerPath); + // Build process arguments explicitly to avoid shell interpretation and command injection. + ProcessBuilder pb = new ProcessBuilder(trustedSlicerPath); + List command = pb.command(); command.add("--load-settings"); - command.add(mFile.getAbsolutePath()); + command.add(machineProfilePath); command.add("--load-settings"); - command.add(pFile.getAbsolutePath()); + command.add(processProfilePath); command.add("--load-filaments"); - command.add(fFile.getAbsolutePath()); + command.add(filamentProfilePath); command.add("--ensure-on-bed"); if (useArrange) { command.add("--arrange"); @@ -102,13 +108,12 @@ public class SlicerService { command.add("--slice"); command.add("0"); command.add("--outputdir"); - command.add(tempDir.toAbsolutePath().toString()); - command.add(inputStl.getAbsolutePath()); + command.add(outputDirPath); + command.add(inputStlPath); logger.info("Executing Slicer" + (useArrange ? " (retry with arrange)" : "") + ": " + String.join(" ", command)); Files.deleteIfExists(slicerLogPath); - ProcessBuilder pb = new ProcessBuilder(command); pb.directory(tempDir.toFile()); pb.redirectErrorStream(true); pb.redirectOutput(slicerLogPath.toFile()); @@ -161,13 +166,13 @@ public class SlicerService { try { tempDir = Files.createTempDirectory("slicer_info_"); Path infoLogPath = tempDir.resolve("orcaslicer-info.log"); + String inputModelPath = requireSafeArgument(inputModel.getAbsolutePath(), "input model path"); - List command = new ArrayList<>(); - command.add(slicerPath); - command.add("--info"); - command.add(inputModel.getAbsolutePath()); - - ProcessBuilder pb = new ProcessBuilder(command); + ProcessBuilder pb = new ProcessBuilder( + trustedSlicerPath, + "--info", + inputModelPath + ); pb.directory(tempDir.toFile()); pb.redirectErrorStream(true); pb.redirectOutput(infoLogPath.toFile()); @@ -267,4 +272,38 @@ public class SlicerService { || normalized.contains("no object is fully inside the print volume") || normalized.contains("calc_exclude_triangles"); } + + private String normalizeExecutablePath(String configuredPath) { + if (configuredPath == null || configuredPath.isBlank()) { + throw new IllegalArgumentException("slicer.path is required"); + } + if (containsControlChars(configuredPath)) { + throw new IllegalArgumentException("slicer.path contains invalid control characters"); + } + try { + return Path.of(configuredPath.trim()).normalize().toString(); + } catch (InvalidPathException e) { + throw new IllegalArgumentException("Invalid slicer.path: " + configuredPath, e); + } + } + + private String requireSafeArgument(String value, String argName) throws IOException { + if (value == null || value.isBlank()) { + throw new IOException("Missing required argument: " + argName); + } + if (containsControlChars(value)) { + throw new IOException("Invalid control characters in " + argName); + } + return value; + } + + private boolean containsControlChars(String value) { + for (int i = 0; i < value.length(); i++) { + char ch = value.charAt(i); + if (ch == '\0' || ch == '\n' || ch == '\r') { + return true; + } + } + return false; + } }