diff --git a/files/pom.xml b/files/pom.xml
index deb3944..ad1aabe 100644
--- a/files/pom.xml
+++ b/files/pom.xml
@@ -68,6 +68,11 @@
+
+ junit
+ junit
+ test
+
\ No newline at end of file
diff --git a/files/src/main/java/de/nbscloud/files/FileSystemService.java b/files/src/main/java/de/nbscloud/files/FileSystemService.java
index 98677b9..268068e 100644
--- a/files/src/main/java/de/nbscloud/files/FileSystemService.java
+++ b/files/src/main/java/de/nbscloud/files/FileSystemService.java
@@ -1,7 +1,6 @@
package de.nbscloud.files;
import de.nbscloud.files.config.FilesConfig;
-import de.nbscloud.files.controller.FilesController;
import de.nbscloud.files.exception.FileSystemServiceException;
import org.apache.commons.io.FileUtils;
import org.apache.commons.lang3.StringUtils;
@@ -89,7 +88,7 @@ public class FileSystemService {
public Path createDirectory(String name) {
try {
- return Files.createDirectories(this.locationTracker.getCurrentLocation().resolve(name));
+ return Files.createDirectories(this.locationTracker.resolve(name));
} catch (IOException e) {
throw new FileSystemServiceException("Could not create directory", e);
}
@@ -97,8 +96,7 @@ public class FileSystemService {
public void createFile(String name, byte[] content) {
try {
- Files.write(this.locationTracker.getCurrentLocation()
- .resolve(name), content, StandardOpenOption.WRITE, StandardOpenOption.CREATE_NEW);
+ Files.write(this.locationTracker.resolve(name), content, StandardOpenOption.WRITE, StandardOpenOption.CREATE_NEW);
} catch (IOException e) {
throw new FileSystemServiceException("Could not create file", e);
}
@@ -112,15 +110,6 @@ public class FileSystemService {
}
}
- public boolean delete(String name) {
- try {
- // TODO does only delete dirs if they are empty - but maybe we want that?
- return Files.deleteIfExists(this.locationTracker.getCurrentLocation().resolve(name));
- } catch (IOException e) {
- throw new FileSystemServiceException("Could not delete file", e);
- }
- }
-
public Path move(Path originalPath, Path newPath) {
try {
return Files.move(originalPath, newPath, StandardCopyOption.ATOMIC_MOVE);
@@ -129,9 +118,18 @@ public class FileSystemService {
}
}
- public byte[] get(String name) {
+ public boolean delete(String name) {
try {
- return Files.readAllBytes(this.locationTracker.getCurrentLocation().resolve(name));
+ // TODO does only delete dirs if they are empty - but maybe we want that?
+ return Files.deleteIfExists(this.locationTracker.resolve(name));
+ } catch (IOException e) {
+ throw new FileSystemServiceException("Could not delete file", e);
+ }
+ }
+
+ public byte[] get(Path path) {
+ try {
+ return Files.readAllBytes(path);
} catch (IOException e) {
throw new FileSystemServiceException("Could not get file", e);
}
@@ -139,7 +137,7 @@ public class FileSystemService {
public void stream(String name, OutputStream outputStream) {
try {
- Files.copy(this.locationTracker.getCurrentLocation().resolve(name), outputStream);
+ Files.copy(this.locationTracker.resolve(name), outputStream);
} catch (IOException e) {
throw new FileSystemServiceException("Could not get file", e);
}
@@ -147,7 +145,7 @@ public class FileSystemService {
public InputStream stream(String name) {
try {
- return Files.newInputStream(this.locationTracker.getCurrentLocation().resolve(name));
+ return Files.newInputStream(this.locationTracker.resolve(name));
} catch (IOException e) {
throw new FileSystemServiceException("Could not get file", e);
}
@@ -155,7 +153,7 @@ public class FileSystemService {
public long getSize(String name) {
try {
- return Files.size(this.locationTracker.getCurrentLocation().resolve(name));
+ return Files.size(this.locationTracker.resolve(name));
} catch (IOException e) {
throw new FileSystemServiceException("Could not get file", e);
}
@@ -163,8 +161,7 @@ public class FileSystemService {
public String getMimeType(String name) {
try {
- final String detectedMimeType = this.mimeTypeDetector.detectMimeType(this.locationTracker.getCurrentLocation()
- .resolve(name));
+ final String detectedMimeType = this.mimeTypeDetector.detectMimeType(this.locationTracker.resolve(name));
logger.debug("Detected mime type {} for file {}", detectedMimeType, name);
@@ -237,7 +234,7 @@ public class FileSystemService {
public List collectDirs(String sourceFile) {
try {
final List resultList = new ArrayList<>();
- final Path sourcePath = this.locationTracker.getCurrentLocation().resolve(sourceFile);
+ final Path sourcePath = this.locationTracker.resolve(sourceFile);
final boolean sourceIsDir = Files.isDirectory(sourcePath);
Files.walkFileTree(this.locationTracker.getBaseDirPath(), new SimpleFileVisitor() {
diff --git a/files/src/main/java/de/nbscloud/files/LocationTracker.java b/files/src/main/java/de/nbscloud/files/LocationTracker.java
index ded9ff4..cbb2cb2 100644
--- a/files/src/main/java/de/nbscloud/files/LocationTracker.java
+++ b/files/src/main/java/de/nbscloud/files/LocationTracker.java
@@ -1,7 +1,6 @@
package de.nbscloud.files;
import de.nbscloud.files.config.FilesConfig;
-import de.nbscloud.files.controller.FilesController;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
@@ -20,6 +19,12 @@ public class LocationTracker {
private Path baseDirPath;
private Path currentLocation;
+ // For testing
+ void init_internal(String baseDir) {
+ this.baseDirPath = Paths.get(baseDir);
+ this.currentLocation = this.baseDirPath.resolve("");
+ }
+
public void init() {
this.baseDirPath = Paths.get(this.filesConfig.getBaseDir());
this.currentLocation = this.baseDirPath.resolve("");
@@ -33,19 +38,19 @@ public class LocationTracker {
init();
}
- public Path getCurrentLocation() {
+ Path getCurrentLocation() {
return this.currentLocation;
}
- public Path getRelativeLocation() {
- return this.baseDirPath.relativize(this.currentLocation);
+ public String getRelativeLocation() {
+ return this.baseDirPath.relativize(this.currentLocation).toString();
}
public Path getRelativeToBaseDir(Path other) {
return this.baseDirPath.relativize(other);
}
- public Path getTrashBin() {
+ private Path getTrashBin() {
return this.baseDirPath.resolve(this.filesConfig.getTrashBinName());
}
@@ -80,12 +85,42 @@ public class LocationTracker {
throw new IllegalStateException("Absolute path: " + navigateTo);
}
- if(!this.baseDirPath.resolve(navigateTo).startsWith(this.baseDirPath)) {
+ // Regardless of where we navigate to, we must never leave the base directory
+ // The normalize() call is important, as it resolves (possible) relative paths
+ if(!this.currentLocation.resolve(navigateTo).normalize().startsWith(this.baseDirPath)) {
+ throw new IllegalStateException("Illegal path: " + navigateTo);
+ }
+
+ if(!this.baseDirPath.resolve(navigateTo).normalize().startsWith(this.baseDirPath)) {
throw new IllegalStateException("Illegal path: " + navigateTo);
}
}
- public Path getBaseDirPath() {
- return baseDirPath;
+ public Path resolve(String name) {
+ validate(name);
+
+ return this.currentLocation.resolve(name).normalize();
+ }
+
+ public Path resolveToBaseDir(String name) {
+ validate(name);
+
+ return this.baseDirPath.resolve(name).normalize();
+ }
+
+ public Path resolveShare(String shareUuid) {
+ validate(shareUuid);
+
+ return this.baseDirPath.resolve(this.filesConfig.getSharesName()).resolve(shareUuid);
+ }
+
+ public Path resolveTrash(String name) {
+ validate(name);
+
+ return this.baseDirPath.resolve(this.filesConfig.getTrashBinName()).resolve(name);
+ }
+
+ Path getBaseDirPath() {
+ return this.baseDirPath;
}
}
diff --git a/files/src/main/java/de/nbscloud/files/controller/FilesController.java b/files/src/main/java/de/nbscloud/files/controller/FilesController.java
index 391b349..e68fcfb 100644
--- a/files/src/main/java/de/nbscloud/files/controller/FilesController.java
+++ b/files/src/main/java/de/nbscloud/files/controller/FilesController.java
@@ -82,8 +82,8 @@ public class FilesController implements InitializingBean {
try {
// Soft delete
this.fileSystemService.move(
- this.locationTracker.getCurrentLocation().resolve(Path.of(filename)),
- this.locationTracker.getTrashBin().resolve(filename));
+ this.locationTracker.resolve(filename),
+ this.locationTracker.resolveTrash(filename));
this.infoMessages.add("nbscloud.files.delete.success");
} catch (RuntimeException e) {
@@ -114,12 +114,12 @@ public class FilesController implements InitializingBean {
@PostMapping("/files/doRename")
public String doRename(RenameForm form) {
- final Path sourcePath = this.locationTracker.getCurrentLocation().resolve(Path.of(form.getOriginalFilename()));
+ final Path sourcePath = this.locationTracker.resolve(form.getOriginalFilename());
// We navigate to the target dir, so we display the content of that dir after the move
this.locationTracker.setCurrentLocation(form.getTargetDir().substring(1));
- final Path targetPath = this.locationTracker.getCurrentLocation().resolve(Path.of(form.getFilename()));
+ final Path targetPath = this.locationTracker.resolve(form.getFilename());
try {
this.fileSystemService.move(sourcePath, targetPath);
@@ -158,7 +158,7 @@ public class FilesController implements InitializingBean {
this.fileSystemService.createFile(file.getOriginalFilename(), file.getBytes());
this.infoMessages.add("nbscloud.files.created.file.success");
- } catch (IOException e) {
+ } catch (IOException | RuntimeException e) {
logger.error("Could not upload file", e);
this.errors.add(e.getMessage());
@@ -185,15 +185,11 @@ public class FilesController implements InitializingBean {
@GetMapping("/files/share")
public String share(String filename) {
- final Path filePath = this.locationTracker.getRelativeToBaseDir(this.locationTracker.getCurrentLocation()
- .resolve(filename));
+ final Path filePath = this.locationTracker.getRelativeToBaseDir(this.locationTracker.resolve(filename));
final String shareUuid = UUID.randomUUID().toString();
try {
- this.fileSystemService.createFile(this.locationTracker.getBaseDirPath()
- .resolve(this.filesConfig.getSharesName())
- .resolve(shareUuid), filePath.toString()
- .getBytes(StandardCharsets.UTF_8));
+ this.fileSystemService.createFile(this.locationTracker.resolveShare(shareUuid), filePath.toString().getBytes(StandardCharsets.UTF_8));
this.shareInfo.add("/files/shares?shareUuid=" + shareUuid);
} catch (RuntimeException e) {
@@ -209,11 +205,8 @@ public class FilesController implements InitializingBean {
@GetMapping("files/shares")
public ResponseEntity shares(String shareUuid) {
try {
- final String sharedFile = new String(this.fileSystemService.get(this.locationTracker.getBaseDirPath()
- .resolve(this.filesConfig.getSharesName())
- .resolve(shareUuid)
- .toString()));
- final Path sharedFilePath = this.locationTracker.getBaseDirPath().resolve(sharedFile);
+ final String sharedFile = new String(this.fileSystemService.get(this.locationTracker.resolveShare(shareUuid)));
+ final Path sharedFilePath = this.locationTracker.resolveToBaseDir(sharedFile);
final String filename = sharedFilePath.getFileName().toString();
return ResponseEntity.ok()
@@ -295,19 +288,19 @@ public class FilesController implements InitializingBean {
}
private String getCurrentLocation() {
- if (this.locationTracker.getRelativeLocation().toString().isEmpty()) {
+ if (this.locationTracker.getRelativeLocation().isEmpty()) {
return "/";
}
- return this.locationTracker.getRelativeLocation().toString();
+ return this.locationTracker.getRelativeLocation();
}
private String getCurrentLocationPrefixed() {
- if (this.locationTracker.getRelativeLocation().toString().isEmpty()) {
+ if (this.locationTracker.getRelativeLocation().isEmpty()) {
return "/";
}
- return "/" + this.locationTracker.getRelativeLocation().toString();
+ return "/" + this.locationTracker.getRelativeLocation();
}
@Override
diff --git a/files/src/test/java/de/nbscloud/files/LocationTrackerTest.java b/files/src/test/java/de/nbscloud/files/LocationTrackerTest.java
new file mode 100644
index 0000000..693a123
--- /dev/null
+++ b/files/src/test/java/de/nbscloud/files/LocationTrackerTest.java
@@ -0,0 +1,75 @@
+package de.nbscloud.files;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public class LocationTrackerTest {
+ private static final String BASE_DIR = "/tmp";
+
+ private LocationTracker locationTracker = new LocationTracker();
+
+ @Before
+ public void init() {
+ this.locationTracker.init_internal(BASE_DIR);
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void test_null() {
+ this.locationTracker.resolve(null);
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void test_relative1() {
+ this.locationTracker.resolve("../test");
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void test_relative2() {
+ this.locationTracker.resolve("/../test");
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void test_relative3() {
+ this.locationTracker.resolve("\\../test");
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void test_relative4() {
+ this.locationTracker.resolve("\\/../test");
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void test_relative5() {
+ this.locationTracker.resolve("..");
+ }
+
+ @Test
+ public void test_relative6_ok() {
+ // While 0x2E translates to the dot character and that might be used to
+ // traverse the path, the Java Path API does not resolve those and just treats them as
+ // any other file/dir name
+ Assert.assertTrue(this.locationTracker.resolve("%2e%2e%2f").toString().equals(BASE_DIR + "/%2e%2e%2f"));
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void test_relative7() {
+ this.locationTracker.resolve("...");
+ }
+
+ @Test
+ public void test_relative8_ok() {
+ Assert.assertTrue(this.locationTracker.resolve("././").toString().equals(BASE_DIR));
+ }
+
+ @Test
+ public void test_relative9_ok() {
+ // The tilde is treated as regular file/dir name if it is resolved from a path
+ Assert.assertTrue(this.locationTracker.resolve("~").toString().equals(BASE_DIR + "/~"));
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void test_absolut() {
+ this.locationTracker.resolve("/bin");
+ }
+}
diff --git a/pom.xml b/pom.xml
index 382e3de..cb52198 100644
--- a/pom.xml
+++ b/pom.xml
@@ -1,7 +1,6 @@
4.0.0
-
org.springframework.boot
spring-boot-starter-parent
@@ -114,6 +113,13 @@
mime-types
1.0.2
+
+
+ junit
+ junit
+ 4.12
+ test
+
@@ -193,5 +199,4 @@
-
-
+
\ No newline at end of file