From 5f1e86c17c98ca6c4c9b0e0cb7fe3cb8db7ff919 Mon Sep 17 00:00:00 2001
From: Conal Smith <68279385+conalsmith-r3@users.noreply.github.com>
Date: Fri, 25 Feb 2022 15:51:49 +0000
Subject: [PATCH] ENT-6508 - Prevent directory traversal from zip file names
 (#7085)

---
 .../persistence/NodeAttachmentService.kt      |  3 ++
 .../persistence/NodeAttachmentServiceTest.kt  | 31 +++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/node/src/main/kotlin/net/corda/node/services/persistence/NodeAttachmentService.kt b/node/src/main/kotlin/net/corda/node/services/persistence/NodeAttachmentService.kt
index e6aee94e7f..6ea173ef4d 100644
--- a/node/src/main/kotlin/net/corda/node/services/persistence/NodeAttachmentService.kt
+++ b/node/src/main/kotlin/net/corda/node/services/persistence/NodeAttachmentService.kt
@@ -88,6 +88,9 @@ class NodeAttachmentService @JvmOverloads constructor(
 
             while (true) {
                 val cursor = jar.nextJarEntry ?: break
+                // Security check to stop directory traversal from filename entry
+                require(!(cursor.name.contains("../"))) { "Bad character in ${cursor.name}" }
+                require(!(cursor.name.contains("..\\"))) { "Bad character in ${cursor.name}" }
                 if (manifestHasEntries && !allManifestEntries!!.remove(cursor.name)) extraFilesNotFoundInEntries.add(cursor)
                 val entryPath = Paths.get(cursor.name)
                 // Security check to stop zips trying to escape their rightful place.
diff --git a/node/src/test/kotlin/net/corda/node/services/persistence/NodeAttachmentServiceTest.kt b/node/src/test/kotlin/net/corda/node/services/persistence/NodeAttachmentServiceTest.kt
index e419df5d01..7c52587a3f 100644
--- a/node/src/test/kotlin/net/corda/node/services/persistence/NodeAttachmentServiceTest.kt
+++ b/node/src/test/kotlin/net/corda/node/services/persistence/NodeAttachmentServiceTest.kt
@@ -46,14 +46,19 @@ import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Ignore
 import org.junit.Test
+import java.io.ByteArrayInputStream
 import java.io.ByteArrayOutputStream
+import java.io.InputStream
 import java.net.URL
 import java.nio.charset.StandardCharsets
 import java.nio.file.FileAlreadyExistsException
 import java.nio.file.FileSystem
 import java.nio.file.Path
 import java.util.*
+import java.util.jar.JarEntry
 import java.util.jar.JarInputStream
+import java.util.jar.JarOutputStream
+import java.util.jar.Manifest
 import kotlin.streams.toList
 import kotlin.test.*
 
@@ -788,6 +793,32 @@ class NodeAttachmentServiceTest {
         }
     }
 
+    @Test(timeout=300_000)
+    fun `attachments containing jar entries whose names expose malicious directory traversal are prevented`() {
+
+        fun createJarWithJarEntryTraversalAttack(jarEntryName: String): InputStream {
+            val byteArrayOutputStream = ByteArrayOutputStream()
+            JarOutputStream(byteArrayOutputStream, Manifest()).apply {
+                putNextEntry(JarEntry(jarEntryName))
+                write("some-text".toByteArray())
+                closeEntry()
+                close()
+            }
+            return ByteArrayInputStream(byteArrayOutputStream.toByteArray())
+        }
+
+        val traversalAttackJarWin = createJarWithJarEntryTraversalAttack("..\\attack")
+        val traversalAttackJarUnix = createJarWithJarEntryTraversalAttack("../attack")
+
+        assertFailsWith(IllegalArgumentException::class) {
+            NodeAttachmentService.checkIsAValidJAR(traversalAttackJarWin)
+        }
+
+        assertFailsWith(IllegalArgumentException::class) {
+            NodeAttachmentService.checkIsAValidJAR(traversalAttackJarUnix)
+        }
+    }
+
     @Test(timeout=300_000)
 	fun `attachments can be queried by providing a intersection of signers using an EQUAL statement - EQUAL containing a single public key`() {
         SelfCleaningDir().use { file ->