Insecure usage of java.nio.file.Path

Imagine you need a method to delete a file based on a parameter (for example: the filename)

The folder structure is as following:

• src

  • main

    • resources

      • file: bad.txt

      • allowed

        • good.txt

Then you could write something such as:

def deleteFile(fileName: String) = {
    val path = java.nio.file.Path.of("src/main/resources/allowed")
    val target = path.resolve(fileName)
    target.toFile.delete()
  }

This looks fine, you can call it as normal:

deleteFile("good.txt")

And this will work fine.

However! This is insecure, because although “resolve” checks for illegal characters in a path, it does not check for path traversal. So the following call can also be made:

deleteFile("../bad.txt")

And the file that should not get deleted (or even worse calling it on the folder “allowed” itself) gets deleted.

The easy solution:

@throws[IllegalArgumentException]
  def deleteFile(fileName: String) = {
    val path = java.nio.file.Path.of("src/main/resources/allowed")
    val target = path.resolve(fileName)

    if(!fileName.contains('/') && target.toFile.isFile) 
      target.toFile.delete()
    else
      throw new IllegalArgumentException(fileName) 
  }

This checks both to see that the filename does not contain a ‘/’ which might indicate an attempt to path traversal, and to check that the file being deleted is a file and not a folder.

Further solving the issue could for example be (our code is not complete in my opinion):

def deleteFile(fileName: String): Try[String] = Try {
    // Validate filename
    if (fileName.contains('/'))
      throw new IllegalArgumentException(s"Illegal attempt to delete file: $fileName")

    // Resolve file path
    val basePath: Path = Paths.get("src/main/resources/allowed")
    val targetPath: Path = basePath.resolve(fileName)

    // Ensure target is a file
    val targetFile = targetPath.toFile
    if (!targetFile.exists() || !targetFile.isFile)
      throw new IllegalArgumentException(s"No such file: $fileName")

    // Delete the file
    if (targetFile.delete()) s"File $fileName deleted successfully."
    else throw new IllegalArgumentException(s"Failed to delete file: $fileName")
}

This will make it fully functional, catching all possible cases and allowing for pattern matching when calling the function.

I know there’s a lot going on so let’s walk over it:

1. We check that the filename does not contain any illegal characters

2. We resolve the file path (and this also checks for other characters that should not be there)

3. Then we check is the target exists and if it’s a file

4. Then we delete the file, and check the deletion

This way we avoid path traversal and deletion of folders, and finally check if the file properly got deleted.

In this case we throw a lot of exceptions, but it’s already functional and more useable.

If you want to be able to pattern match more on the result you can go further (and I’m going way off-topic):

sealed trait FileDeletionOperation
case object FileDeletionOperationSuccess extends FileDeletionOperation
case object FileDeletionOperationIllegalPath extends FileDeletionOperation
case object FileDeletionOperationNoSuchFile extends FileDeletionOperation
case object FileDeletionOperationNotDeleted extends FileDeletionOperation

def deleteFile(fileName: String): FileDeletionOperation = {
  if (fileName.contains('/')) FileDeletionOperationIllegalPath else
    val basePath: Path = Paths.get("src/main/resources/allowed")
    val targetPath: Path = basePath.resolve(fileName)
    val targetFile = targetPath.toFile
    if (!targetFile.exists() || !targetFile.isFile) FileDeletionOperationNoSuchFile
    else 
      if (targetFile.delete()) FileDeletionOperationSuccess 
      else FileDeletionOperationNotDeleted
  }

//calling it
val f = (s: String) => deleteFile(s) match
      case FileDeletionOperationSuccess => println(s"File ${s} deleted successfully")
      case FileDeletionOperationIllegalPath => println(s"File ${s} contains illegal path args")
      case FileDeletionOperationNoSuchFile => println(s"File ${s} does not exist or conform")
      case FileDeletionOperationNotDeleted => println(s"File ${s} not deleted")


f("good.txt")
f("../bad.txt")

/*
Console:
    File good.txt deleted successfully
    File ../bad.txt contains illegal path args
*/

That’s all folks :)