fix(deploy): fix security
This commit is contained in:
@@ -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<byte[]> 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");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<String> command = new ArrayList<>();
|
||||
command.add(slicerPath);
|
||||
// Build process arguments explicitly to avoid shell interpretation and command injection.
|
||||
ProcessBuilder pb = new ProcessBuilder(trustedSlicerPath);
|
||||
List<String> 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<String> 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;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user