Merge pull request #255 from hotwired/path-traversal-fix

Prevent path traversal vulnerability in TurboUriHelper

turbo/src/main/kotlin/dev/hotwire/turbo/util/TurboUriHelper.kt

@@ -11,15 +11,20 @@ import java.io.IOException
 
 internal class TurboUriHelper(val context: Context) {
     @Suppress("BlockingMethodInNonBlockingContext") // https://youtrack.jetbrains.com/issue/KT-39684
-    suspend fun writeFileTo(uri: Uri, directory: File): File? {
+    suspend fun writeFileTo(uri: Uri, destDirectory: File): File? {
         val uriAttributes = getAttributes(uri) ?: return null
+        val file = File(destDirectory, uriAttributes.fileName)
 
-        return withContext(dispatcherProvider.io) {
-            val file = File(directory, uriAttributes.fileName).also {
-                if (it.exists()) it.delete()
-            }
+        if (file.hasPathTraversalVulnerability(destDirectory)) {
+            return null
+        }
 
+        return withContext(dispatcherProvider.io) {
             try {
+                if (file.exists()) {
+                    file.delete()
+                }
+
                 context.contentResolver.openInputStream(uri).use {
                     val outputStream = file.outputStream()
                     it?.copyTo(outputStream)
@@ -104,6 +109,24 @@ internal class TurboUriHelper(val context: Context) {
             fileSize = 0
         )
     }
+
+    /**
+     * Checks for path traversal vulnerability (caused e.g. by input filename containing "../")
+     * which could lead to modification of the destination directory.
+     *
+     * More information: https://developer.android.com/topic/security/risks/path-traversal
+     */
+    private fun File.hasPathTraversalVulnerability(destDirectory: File): Boolean {
+        return try {
+            val destinationDirectoryPath = destDirectory.canonicalPath
+            val outputFilePath = this.canonicalPath
+
+            !outputFilePath.startsWith(destinationDirectoryPath)
+        } catch (e: Exception) {
+            TurboLog.e("${e.message}")
+            false
+        }
+    }
 
     /**
      * Determine if the file points to an app resource. Symbolic link

turbo/src/test/kotlin/dev/hotwire/turbo/util/TurboUriHelperTest.kt

@@ -0,0 +1,79 @@
+package dev.hotwire.turbo.util
+
+import android.content.Context
+import android.net.Uri
+import android.os.Build
+import androidx.test.core.app.ApplicationProvider
+import dev.hotwire.turbo.BaseUnitTest
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.TestCoroutineDispatcher
+import kotlinx.coroutines.test.resetMain
+import kotlinx.coroutines.test.runTest
+import kotlinx.coroutines.test.setMain
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.Shadows
+import org.robolectric.annotation.Config
+import java.io.File
+
+@ExperimentalCoroutinesApi
+@RunWith(RobolectricTestRunner::class)
+@Config(sdk = [Build.VERSION_CODES.O])
+class TurboUriHelperTest : BaseUnitTest() {
+
+    private val testDispatcher: TestCoroutineDispatcher = TestCoroutineDispatcher()
+
+    private lateinit var context: Context
+    private lateinit var turboUriHelper: TurboUriHelper
+
+    @Before
+    override fun setup() {
+        super.setup()
+        Dispatchers.setMain(testDispatcher)
+        dispatcherProvider.io = Dispatchers.Main
+
+        context = ApplicationProvider.getApplicationContext()
+        turboUriHelper = TurboUriHelper(context)
+    }
+
+    override fun teardown() {
+        super.teardown()
+        Dispatchers.resetMain()
+        testDispatcher.cleanupTestCoroutines()
+    }
+
+    @Test
+    fun validUriIsWrittenToFileSuccessfully() = runTest {
+        val inputFile = File("/tmp/file.txt")
+        val inputFileUri = Uri.fromFile(inputFile)
+        Shadows.shadowOf(context.contentResolver).registerInputStream(inputFileUri, "fileContent".byteInputStream())
+
+        val destFile = turboUriHelper.writeFileTo(inputFileUri, TurboFileProvider.directory(context))
+
+        assertThat(destFile).isNotNull()
+    }
+
+    @Test
+    fun pathTraversingUriWithRelativePathFailsToWriteToFile() = runTest {
+        val inputFileUri = Uri.parse("../../tmp/file.txt")
+        Shadows.shadowOf(context.contentResolver).registerInputStream(inputFileUri, "fileContent".byteInputStream())
+
+        val destFile = turboUriHelper.writeFileTo(inputFileUri, TurboFileProvider.directory(context))
+
+        assertThat(destFile).isNull()
+    }
+
+    @Test
+    fun pathTraversingUriWithNameArgFailsToWriteToFile() = runTest {
+        val inputFileUri = Uri.parse("content://malicious.app?path=/data/data/malicious.app/files/file.txt&name=../../file.txt")
+        Shadows.shadowOf(context.contentResolver).registerInputStream(inputFileUri, "fileContent".byteInputStream())
+
+        val destFile = turboUriHelper.writeFileTo(inputFileUri, TurboFileProvider.directory(context))
+
+        assertThat(destFile).isNull()
+    }
+}