ColorRecipesExplorer
/
Backend
1
1
Fork 0
Code Issues 7 Pull Requests Projects 1 Releases 2 Wiki Activity

Ajout des certaines chaines de caractères bannies dans le chemin des fichiers dans FileService.

This commit is contained in:
FyloZ 2021-04-28 14:27:29 -04:00
parent 361b1b2ba3
commit 64829f74cb
3 changed files with 67 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/main/kotlin/dev/fyloz/colorrecipesexplorer/rest/files/FileController.kt
View File

@ -12,6 +12,7 @@ import org.springframework.web.multipart.MultipartFile
import java.net.URI
const val FILE_CONTROLLER_PATH = "/api/file"
private const val DEFAULT_MEDIA_TYPE = MediaType.APPLICATION_OCTET_STREAM_VALUE
@RestController
@RequestMapping(FILE_CONTROLLER_PATH)
@ -21,12 +22,15 @@ class FileController(
) {
@GetMapping(produces = [MediaType.APPLICATION_OCTET_STREAM_VALUE])
@PreAuthorize("hasAnyAuthority('READ_FILE')")
fun upload(@RequestParam path: String): ResponseEntity<ByteArrayResource> {
fun upload(
@RequestParam path: String,
@RequestParam(required = false) mediaType: String?
): ResponseEntity<ByteArrayResource> {
val file = fileService.read(path)
return ResponseEntity.ok()
.header("Content-Disposition", "attachment; filename=${getFileNameFromPath(path)}")
.header("Content-Disposition", "filename=${getFileNameFromPath(path)}")
.contentLength(file.contentLength())
.contentType(MediaType.APPLICATION_OCTET_STREAM)
.contentType(MediaType.parseMediaType(mediaType ?: DEFAULT_MEDIA_TYPE))
.body(file)
}

28
src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileService.kt
View File

@ -11,6 +11,13 @@ import java.io.File
import java.io.IOException
import java.nio.file.Files
/** Banned path shards. These are banned because they can allow access to files outside the data directory. */
val BANNED_FILE_PATH_SHARDS = setOf(
"~",
"..",
"//"
)
interface FileService {
/** Checks if the file at the given [path] exists. */
fun exists(path: String): Boolean
@ -93,8 +100,13 @@ class FileServiceImpl(
}
}
override fun String.fullPath() =
FilePath("${creProperties.workingDirectory}/$this")
override fun String.fullPath(): FilePath {
BANNED_FILE_PATH_SHARDS
.firstOrNull { this.contains(it) }
?.let { throw InvalidFilePathException(this, it) }
return FilePath("${creProperties.workingDirectory}/$this")
}
/** Runs the given [block] in the context of a file with the given [fullPath]. */
private fun <T> withFileAt(fullPath: FilePath, block: File.() -> T) =
@ -114,6 +126,18 @@ fun File.create() {
private const val FILE_IO_EXCEPTION_TITLE = "File IO error"
class InvalidFilePathException(val path: String, val fragment: String) :
RestException(
"invalid-filepath",
"Invalid file path",
HttpStatus.BAD_REQUEST,
"The given path is invalid because it contains a potentially malicious String '$fragment'",
mapOf(
"path" to path,
"invalidString" to fragment
)
)
class FileExistsException(val path: String) :
RestException(
"io-exists",

52
src/test/kotlin/dev/fyloz/colorrecipesexplorer/service/files/FileServiceTest.kt
View File

@ -51,14 +51,14 @@ class FileServiceTest {
@Test
fun `exists() returns true when the file at the given path exists and is a file`() {
fileServiceTest {
test {
assertTrue { fileService.exists(mockFilePath) }
}
}
@Test
fun `exists() returns false when the file at the given path does not exist`() {
fileServiceTest {
test {
every { mockFile.exists() } returns false
assertFalse { fileService.exists(mockFilePath) }
@ -67,7 +67,7 @@ class FileServiceTest {
@Test
fun `exists() returns false when the file at the given path is not a file`() {
fileServiceTest {
test {
every { mockFile.isFile } returns false
assertFalse { fileService.exists(mockFilePath) }
@ -78,7 +78,7 @@ class FileServiceTest {
@Test
fun `read() returns a valid ByteArrayResource`() {
fileServiceTest {
test {
whenMockFilePathExists {
mockkStatic(File::readBytes)
every { mockFile.readBytes() } returns mockFileData
@ -92,7 +92,7 @@ class FileServiceTest {
@Test
fun `read() throws FileNotFoundException when no file exists at the given path`() {
fileServiceTest {
test {
whenMockFilePathExists(false) {
with(assertThrows<FileNotFoundException> { fileService.read(mockFilePath) }) {
assertEquals(mockFilePath, this.path)
@ -103,7 +103,7 @@ class FileServiceTest {
@Test
fun `read() throws FileReadException when an IOException is thrown`() {
fileServiceTest {
test {
whenMockFilePathExists {
mockkStatic(File::readBytes)
every { mockFile.readBytes() } throws IOException()
@ -119,7 +119,7 @@ class FileServiceTest {
@Test
fun `create() creates a file at the given path`() {
fileServiceTest {
test {
whenMockFilePathExists(false) {
mockkStatic(File::create)
every { mockFile.create() } just Runs
@ -135,7 +135,7 @@ class FileServiceTest {
@Test
fun `create() does nothing when a file already exists at the given path`() {
fileServiceTest {
test {
whenMockFilePathExists {
fileService.create(mockFilePath)
@ -148,7 +148,7 @@ class FileServiceTest {
@Test
fun `create() throws FileCreateException when the file creation throws an IOException`() {
fileServiceTest {
test {
whenMockFilePathExists(false) {
mockkStatic(File::create)
every { mockFile.create() } throws IOException()
@ -164,7 +164,7 @@ class FileServiceTest {
@Test
fun `write() creates and writes the given MultipartFile to the file at the given path`() {
fileServiceTest {
test {
whenMockFilePathExists(false) {
every { fileService.create(mockFilePath) } just Runs
every { mockMultipartFile.transferTo(mockFilePathPath) } just Runs
@ -181,7 +181,7 @@ class FileServiceTest {
@Test
fun `write() throws FileExistsException when a file at the given path already exists and overwrite is disabled`() {
fileServiceTest {
test {
whenMockFilePathExists {
with(assertThrows<FileExistsException> { fileService.write(mockMultipartFile, mockFilePath, false) }) {
assertEquals(mockFilePath, this.path)
@ -192,7 +192,7 @@ class FileServiceTest {
@Test
fun `write() writes the given MultipartFile to an existing file when overwrite is enabled`() {
fileServiceTest {
test {
whenMockFilePathExists {
every { mockMultipartFile.transferTo(mockFilePathPath) } just Runs
@ -207,7 +207,7 @@ class FileServiceTest {
@Test
fun `write() throws FileWriteException when writing the given file throws an IOException`() {
fileServiceTest {
test {
whenMockFilePathExists(false) {
every { fileService.create(mockFilePath) } just Runs
every { mockMultipartFile.transferTo(mockFilePathPath) } throws IOException()
@ -225,7 +225,7 @@ class FileServiceTest {
@Test
fun `delete() deletes the file at the given path`() {
fileServiceTest {
test {
whenMockFilePathExists {
every { mockFile.delete() } returns true
@ -236,7 +236,7 @@ class FileServiceTest {
@Test
fun `delete() throws FileNotFoundException when no file exists at the given path`() {
fileServiceTest {
test {
whenMockFilePathExists(false) {
with(assertThrows<FileNotFoundException> { fileService.delete(mockFilePath) }) {
assertEquals(mockFilePath, this.path)
@ -247,7 +247,7 @@ class FileServiceTest {
@Test
fun `delete() throws FileDeleteException when deleting throw and IOException`() {
fileServiceTest {
test {
whenMockFilePathExists {
every { mockFile.delete() } throws IOException()
@ -262,7 +262,7 @@ class FileServiceTest {
@Test
fun `fullPath() appends the given path to the given working directory`() {
fileServiceTest {
test {
with(fileService) {
val fullFilePath = mockFilePath.fullPath()
@ -271,7 +271,23 @@ class FileServiceTest {
}
}
private fun fileServiceTest(test: FileServiceTestContext.() -> Unit) {
@Test
fun `fullPath() throws InvalidFilePathException when the given path contains invalid fragments`() {
test {
with(fileService) {
BANNED_FILE_PATH_SHARDS.forEach {
val maliciousPath = "$it/$mockFilePath"
with(assertThrows<InvalidFilePathException> { maliciousPath.fullPath() }) {
assertEquals(maliciousPath, this.path)
assertEquals(it, this.fragment)
}
}
}
}
}
private fun test(test: FileServiceTestContext.() -> Unit) {
FileServiceTestContext().test()
}