diff --git a/files/src/main/java/de/nbscloud/files/FileSystemService.java b/files/src/main/java/de/nbscloud/files/FileSystemService.java index 4e08fd3..7c73de9 100644 --- a/files/src/main/java/de/nbscloud/files/FileSystemService.java +++ b/files/src/main/java/de/nbscloud/files/FileSystemService.java @@ -88,6 +88,7 @@ public class FileSystemService { private MimeTypeDetector mimeTypeDetector = new MimeTypeDetector(); public Path createDirectory(String name) { + try { return Files.createDirectories(this.locationTracker.resolve(name)); } catch (IOException e) { @@ -96,6 +97,8 @@ public class FileSystemService { } void createDirectory(Path path) { + this.locationTracker.ensureValidPath(path); + try { Files.createDirectories(path); } catch (IOException e) { @@ -104,14 +107,12 @@ public class FileSystemService { } public void createFile(String name, byte[] content) { - try { - Files.write(this.locationTracker.resolve(name), content, StandardOpenOption.WRITE, StandardOpenOption.CREATE_NEW); - } catch (IOException e) { - throw new FileSystemServiceException("Could not create file", e); - } + createFile(this.locationTracker.resolve(name), content); } public void createFile(Path path, byte[] content) { + this.locationTracker.ensureValidPath(path); + try { Files.write(path, content, StandardOpenOption.WRITE, StandardOpenOption.CREATE_NEW); } catch (IOException e) { @@ -120,6 +121,8 @@ public class FileSystemService { } public void overwriteFile(Path path, byte[] content) { + this.locationTracker.ensureValidPath(path); + try { Files.write(path, content, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING); } catch (IOException e) { @@ -128,6 +131,9 @@ public class FileSystemService { } public Path move(Path originalPath, Path newPath) { + this.locationTracker.ensureValidPath(originalPath); + this.locationTracker.ensureValidPath(newPath); + try { return Files.move(originalPath, newPath, StandardCopyOption.ATOMIC_MOVE); } catch (IOException e) { @@ -136,15 +142,12 @@ 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.resolve(name)); - } catch (IOException e) { - throw new FileSystemServiceException("Could not delete file", e); - } + return delete(this.locationTracker.resolve(name)); } public boolean delete(Path path) { + this.locationTracker.ensureValidPath(path); + try { // TODO does only delete dirs if they are empty - but maybe we want that? return Files.deleteIfExists(path); @@ -154,6 +157,8 @@ public class FileSystemService { } public byte[] get(Path path) { + this.locationTracker.ensureValidPath(path); + try { return Files.readAllBytes(path); } catch (IOException e) { @@ -170,6 +175,8 @@ public class FileSystemService { } public void stream(Path targetPath, ZipOutputStream outputStream) { + this.locationTracker.ensureValidPath(targetPath); + try { Files.walkFileTree(targetPath, new SimpleFileVisitor() { @Override @@ -198,14 +205,12 @@ public class FileSystemService { } public InputStream stream(String name) { - try { - return Files.newInputStream(this.locationTracker.resolve(name)); - } catch (IOException e) { - throw new FileSystemServiceException("Could not stream file", e); - } + return stream(this.locationTracker.resolve(name)); } - private InputStream stream(Path name) { + public InputStream stream(Path name) { + this.locationTracker.ensureValidPath(name); + try { return Files.newInputStream(name); } catch (IOException e) { @@ -214,18 +219,16 @@ public class FileSystemService { } public long getSize(String name) { - try { - return Files.size(this.locationTracker.resolve(name)); - } catch (IOException e) { - throw new FileSystemServiceException("Could not get file", e); - } + return getSize(this.locationTracker.resolve(name)); } public long getSize(Path name) { + this.locationTracker.ensureValidPath(name); + try { return Files.size(name); } catch (IOException e) { - throw new FileSystemServiceException("Could not get file", e); + throw new FileSystemServiceException("Could not get file size", e); } } @@ -258,10 +261,16 @@ public class FileSystemService { } public String getMimeType(String name) { - try { - final String detectedMimeType = this.mimeTypeDetector.detectMimeType(this.locationTracker.resolve(name)); + return getMimeType(this.locationTracker.resolve(name)); + } - logger.debug("Detected mime type {} for file {}", detectedMimeType, name); + public String getMimeType(Path path) { + this.locationTracker.ensureValidPath(path); + + try { + final String detectedMimeType = this.mimeTypeDetector.detectMimeType(path); + + logger.debug("Detected mime type {} for file {}", detectedMimeType, path); return detectedMimeType; } catch (GetBytesException e) { @@ -295,6 +304,8 @@ public class FileSystemService { } List list(Path startPath, SortOrder sortOrder, Function relativizer) { + this.locationTracker.ensureValidPath(startPath); + try { List contentList = Files.list(startPath) .filter(path -> { @@ -330,6 +341,8 @@ public class FileSystemService { } ContentTree getTree(Path startPath, SortOrder sortOrder, Function relativizer) { + this.locationTracker.ensureValidPath(startPath); + try { if (!Files.isDirectory(startPath)) { return null; @@ -399,6 +412,9 @@ public class FileSystemService { } List collectDirs(Path sourcePath, Path baseDir, Function relativizer, boolean includeSource) { + this.locationTracker.ensureValidPath(sourcePath); + this.locationTracker.ensureValidPath(baseDir); + try { final List resultList = new ArrayList<>(); final boolean sourceIsDir = Files.isDirectory(sourcePath); diff --git a/files/src/main/java/de/nbscloud/files/LocationTracker.java b/files/src/main/java/de/nbscloud/files/LocationTracker.java index 078e077..daf2e95 100644 --- a/files/src/main/java/de/nbscloud/files/LocationTracker.java +++ b/files/src/main/java/de/nbscloud/files/LocationTracker.java @@ -106,6 +106,25 @@ public class LocationTracker implements InitializingBean { } } + public void ensureValidPath(Path path) { + // The provided systemd unit file restricts access to the nbscloud/ directory + // but better safe than sorry - user could roll their own service after all + + if(path == null) { + throw new IllegalStateException("Null"); + } + + if(path.toString().contains("..")) { + throw new IllegalStateException("Relative path"); + } + + // 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(!path.normalize().startsWith(this.baseDirPath)) { + throw new IllegalStateException("Illegal path: " + path); + } + } + public Path resolve(String name) { validate_internal(name); 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 c1d0574..4524558 100644 --- a/files/src/main/java/de/nbscloud/files/controller/FilesController.java +++ b/files/src/main/java/de/nbscloud/files/controller/FilesController.java @@ -344,9 +344,9 @@ public class FilesController implements InitializingBean { return ResponseEntity.ok() .header(HttpHeaders.CONTENT_DISPOSITION, "attachment; filename=" + filename) - .header(HttpHeaders.CONTENT_TYPE, this.fileSystemService.getMimeType(filename)) - .header(HttpHeaders.CONTENT_LENGTH, String.valueOf(this.fileSystemService.getSize(filename))) - .body(new InputStreamResource(new ObservableInputStream(this.fileSystemService.stream(filename), new ObservableInputStream.Observer() { + .header(HttpHeaders.CONTENT_TYPE, this.fileSystemService.getMimeType(sharedFilePath)) + .header(HttpHeaders.CONTENT_LENGTH, String.valueOf(this.fileSystemService.getSize(sharedFilePath))) + .body(new InputStreamResource(new ObservableInputStream(this.fileSystemService.stream(sharedFilePath), new ObservableInputStream.Observer() { @Override public void closed() throws IOException { if (share.isOneTime()) { diff --git a/web-container/src/main/resources/static/changelog.txt b/web-container/src/main/resources/static/changelog.txt index 1470abf..2d070db 100644 --- a/web-container/src/main/resources/static/changelog.txt +++ b/web-container/src/main/resources/static/changelog.txt @@ -1,5 +1,8 @@ +v19: +- #22 Fix a bug with password protected shares + v18: -- Password protected shares +- #22 Password protected shares - Basic Note app implementation - Files app now offers API for other apps to store files