ENT-6330 Fixed reading jar entries in memory (#6960)

* ENT-6330 Fixed reading jar entries in memory

This is a trivial fix that is however enough to allow to send zip bombs as attachments without the node crashing, a size limit could be added for increased reliability

* added attachment cumulative size check

* added compression ratio check

* added unit test and moved the code to a standalone verifier object

* removed attachment check from AttachmentClassLoader to minimize performance impact

core/build.gradle

@@ -110,7 +110,16 @@ configurations {
 }
 
 
-test{
+processTestResources {
+    inputs.files(jar)
+    into("zip") {
+        from(jar) {
+            rename { "core.jar" }
+        }
+    }
+}
+
+test {
     maxParallelForks = (System.env.CORDA_CORE_TESTING_FORKS == null) ? 1 :  "$System.env.CORDA_CORE_TESTING_FORKS".toInteger()
 }
 

core/src/main/kotlin/net/corda/core/contracts/TransactionVerificationException.kt

@@ -343,7 +343,11 @@ abstract class TransactionVerificationException(val txId: SecureHash, message: S
                     "You will need to manually install the CorDapp to whitelist it for use.")
 
     @KeepForDJVM
-    class UnsupportedHashTypeException(txId: SecureHash) : TransactionVerificationException(txId, "The transaction Id is defined by an unsupported hash type", null);
+    class UnsupportedHashTypeException(txId: SecureHash) : TransactionVerificationException(txId, "The transaction Id is defined by an unsupported hash type", null)
+
+    @KeepForDJVM
+    class AttachmentTooBigException(txId: SecureHash) : TransactionVerificationException(
+            txId, "The transaction attachments are too large and exceed both max transaction size and the maximum allowed compression ratio", null)
 
     /*
     If you add a new class extending [TransactionVerificationException], please add a test in `TransactionVerificationExceptionSerializationTests`

core/src/main/kotlin/net/corda/core/internal/utilities/ZipBombDetector.kt

@@ -0,0 +1,63 @@
+package net.corda.core.internal.utilities
+
+import java.io.FilterInputStream
+import java.io.InputStream
+import java.util.zip.ZipInputStream
+
+object ZipBombDetector {
+
+    private class CounterInputStream(source : InputStream) : FilterInputStream(source) {
+        private var byteCount : Long = 0
+
+        val count : Long
+            get() = byteCount
+
+        override fun read(): Int {
+            return super.read().also { byte ->
+                if(byte >= 0) byteCount += 1
+            }
+        }
+
+        override fun read(b: ByteArray): Int {
+            return super.read(b).also { bytesRead ->
+                if(bytesRead > 0) byteCount += bytesRead
+            }
+        }
+
+        override fun read(b: ByteArray, off: Int, len: Int): Int {
+            return super.read(b, off, len).also { bytesRead ->
+                if(bytesRead > 0) byteCount += bytesRead
+            }
+        }
+    }
+
+    /**
+     * Check if a zip file is a potential malicious zip bomb
+     * @param source the zip archive file content
+     * @param maxUncompressedSize the maximum allowable uncompressed archive size
+     * @param maxCompressionRatio the maximum allowable compression ratio
+     * @return true if the zip file total uncompressed size exceeds [maxUncompressedSize] and the
+     * average entry compression ratio is larger than [maxCompressionRatio], false otherwise
+     */
+    @Suppress("NestedBlockDepth")
+    fun scanZip(source : InputStream, maxUncompressedSize : Long, maxCompressionRatio : Float = 10.0f) : Boolean {
+        val counterInputStream = CounterInputStream(source)
+        var uncompressedByteCount : Long = 0
+        val buffer = ByteArray(DEFAULT_BUFFER_SIZE)
+        ZipInputStream(counterInputStream).use { zipInputStream ->
+            while(true) {
+                zipInputStream.nextEntry ?: break
+                while(true) {
+                    val read = zipInputStream.read(buffer)
+                    if(read <= 0) break
+                    uncompressedByteCount += read
+                    if(uncompressedByteCount > maxUncompressedSize &&
+                            uncompressedByteCount.toFloat() / counterInputStream.count.toFloat() > maxCompressionRatio) {
+                        return true
+                    }
+                }
+            }
+        }
+        return false
+    }
+}

core/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsClassLoader.kt

@@ -9,19 +9,35 @@ import net.corda.core.contracts.TransactionVerificationException
 import net.corda.core.contracts.TransactionVerificationException.OverlappingAttachmentsException
 import net.corda.core.contracts.TransactionVerificationException.PackageOwnershipException
 import net.corda.core.crypto.SecureHash
-import net.corda.core.crypto.sha256
-import net.corda.core.internal.*
+import net.corda.core.internal.JDK1_2_CLASS_FILE_FORMAT_MAJOR_VERSION
+import net.corda.core.internal.JDK8_CLASS_FILE_FORMAT_MAJOR_VERSION
+import net.corda.core.internal.JarSignatureCollector
+import net.corda.core.internal.NamedCacheFactory
+import net.corda.core.internal.PlatformVersionSwitches
+import net.corda.core.internal.VisibleForTesting
 import net.corda.core.internal.cordapp.targetPlatformVersion
+import net.corda.core.internal.createInstancesOfClassesImplementing
+import net.corda.core.internal.createSimpleCache
+import net.corda.core.internal.toSynchronised
 import net.corda.core.node.NetworkParameters
-import net.corda.core.serialization.*
+import net.corda.core.serialization.SerializationContext
+import net.corda.core.serialization.SerializationCustomSerializer
+import net.corda.core.serialization.SerializationFactory
+import net.corda.core.serialization.SerializationWhitelist
+import net.corda.core.serialization.SingletonSerializeAsToken
 import net.corda.core.serialization.internal.AttachmentURLStreamHandlerFactory.toUrl
+import net.corda.core.serialization.withWhitelist
 import net.corda.core.utilities.contextLogger
 import net.corda.core.utilities.debug
-import java.io.ByteArrayOutputStream
 import java.io.IOException
 import java.io.InputStream
 import java.lang.ref.WeakReference
-import java.net.*
+import java.net.URL
+import java.net.URLClassLoader
+import java.net.URLConnection
+import java.net.URLStreamHandler
+import java.net.URLStreamHandlerFactory
+import java.security.MessageDigest
 import java.security.Permission
 import java.util.*
 import java.util.function.Function
@@ -128,6 +144,20 @@ class AttachmentsClassLoader(attachments: List<Attachment>,
         checkAttachments(attachments)
     }
 
+    private class AttachmentHashContext(
+            val txId: SecureHash,
+            val buffer: ByteArray = ByteArray(DEFAULT_BUFFER_SIZE))
+
+    private fun hash(inputStream : InputStream, ctx : AttachmentHashContext) : SecureHash.SHA256 {
+        val md = MessageDigest.getInstance(SecureHash.SHA2_256)
+        while(true) {
+            val read = inputStream.read(ctx.buffer)
+            if(read <= 0) break
+            md.update(ctx.buffer, 0, read)
+        }
+        return SecureHash.SHA256(md.digest())
+    }
+
     private fun isZipOrJar(attachment: Attachment) = attachment.openAsJAR().use { jar ->
         jar.nextEntry != null
     }
@@ -160,6 +190,7 @@ class AttachmentsClassLoader(attachments: List<Attachment>,
         }
     }
 
+    @Suppress("ThrowsCount", "ComplexMethod", "NestedBlockDepth")
     private fun checkAttachments(attachments: List<Attachment>) {
         require(attachments.isNotEmpty()) { "attachments list is empty" }
 
@@ -189,6 +220,7 @@ class AttachmentsClassLoader(attachments: List<Attachment>,
         // claim their parts of the Java package namespace via registration with the zone operator.
 
         val classLoaderEntries = mutableMapOf<String, SecureHash.SHA256>()
+        val ctx = AttachmentHashContext(sampleTxId)
         for (attachment in attachments) {
             // We may have been given an attachment loaded from the database in which case, important info like
             // signers is already calculated.
@@ -208,8 +240,10 @@ class AttachmentsClassLoader(attachments: List<Attachment>,
                 // perceived correctness of the signatures or package ownership already, that would be too late.
                 attachment.openAsJAR().use { JarSignatureCollector.collectSigners(it) }
             }
+
             // Now open it again to compute the overlap and package ownership data.
             attachment.openAsJAR().use { jar ->
+
                 val targetPlatformVersion = jar.manifest?.targetPlatformVersion ?: 1
                 while (true) {
                     val entry = jar.nextJarEntry ?: break
@@ -250,13 +284,9 @@ class AttachmentsClassLoader(attachments: List<Attachment>,
                     if (!shouldCheckForNoOverlap(path, targetPlatformVersion)) continue
 
                     // This calculates the hash of the current entry because the JarInputStream returns only the current entry.
-                    fun entryHash() = ByteArrayOutputStream().use {
-                        jar.copyTo(it)
-                        it.toByteArray()
-                    }.sha256()
+                    val currentHash = hash(jar, ctx)
 
                     // If 2 entries are identical, it means the same file is present in both attachments, so that is ok.
-                    val currentHash = entryHash()
                     val previousFileHash = classLoaderEntries[path]
                     when {
                         previousFileHash == null -> {

core/src/test/kotlin/net/corda/core/internal/utilities/ZipBombDetectorTest.kt

@@ -0,0 +1,66 @@
+package net.corda.core.internal.utilities
+
+import org.junit.Assert
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
+
+@RunWith(Parameterized::class)
+class ZipBombDetectorTest(private val case : TestCase) {
+
+    enum class TestCase(
+            val description : String,
+            val zipResource : String,
+            val maxUncompressedSize : Long,
+            val maxCompressionRatio : Float,
+            val expectedOutcome : Boolean
+    ) {
+        LEGIT_JAR("This project's jar file", "zip/core.jar", 128_000, 10f, false),
+
+        // This is not detected as a zip bomb as ZipInputStream is unable to read all of its entries
+        // (https://stackoverflow.com/questions/69286786/zipinputstream-cannot-parse-a-281-tb-zip-bomb),
+        // so the total uncompressed size doesn't exceed maxUncompressedSize
+        SMALL_BOMB(
+                "A large (5.5 GB) zip archive",
+                "zip/zbsm.zip", 64_000_000, 10f, false),
+
+        // Decreasing maxUncompressedSize leads to a successful detection
+        SMALL_BOMB2(
+                "A large (5.5 GB) zip archive, with 1MB maxUncompressedSize",
+                "zip/zbsm.zip", 1_000_000, 10f, true),
+
+        // ZipInputStream is also unable to read all entries of zblg.zip, but since the first one is already bigger than 4GB,
+        // that is enough to exceed maxUncompressedSize
+        LARGE_BOMB(
+            "A huge (281 TB) Zip bomb, this is the biggest possible non-recursive non-Zip64 archive",
+            "zip/zblg.zip", 64_000_000, 10f, true),
+
+        //Same for this, but its entries are 22GB each
+        EXTRA_LARGE_BOMB(
+            "A humongous (4.5 PB) Zip64 bomb",
+            "zip/zbxl.zip", 64_000_000, 10f, true),
+
+        //This is a jar file containing a single 10GB manifest
+        BIG_MANIFEST(
+                "A jar file with a huge manifest",
+                "zip/big-manifest.jar", 64_000_000, 10f, true);
+
+        override fun toString() = description
+    }
+
+    companion object {
+        @JvmStatic
+        @Parameterized.Parameters(name = "{0}")
+        fun primeNumbers(): Collection<*> {
+            return TestCase.values().toList()
+        }
+    }
+
+    @Test(timeout=10_000)
+    fun test() {
+        (javaClass.classLoader.getResourceAsStream(case.zipResource) ?:
+            throw IllegalStateException("Missing test resource file ${case.zipResource}")).let {
+            Assert.assertEquals(case.expectedOutcome, ZipBombDetector.scanZip(it, case.maxUncompressedSize, case.maxCompressionRatio))
+        }
+    }
+}

node/src/main/kotlin/net/corda/node/services/persistence/NodeAttachmentService.kt

@@ -16,6 +16,7 @@ import net.corda.core.internal.*
 import net.corda.core.internal.Version
 import net.corda.core.internal.cordapp.CordappImpl.Companion.CORDAPP_CONTRACT_VERSION
 import net.corda.core.internal.cordapp.CordappImpl.Companion.DEFAULT_CORDAPP_VERSION
+import net.corda.core.internal.utilities.ZipBombDetector
 import net.corda.core.node.ServicesForResolution
 import net.corda.core.node.services.AttachmentId
 import net.corda.core.node.services.vault.AttachmentQueryCriteria
@@ -33,6 +34,7 @@ import net.corda.nodeapi.internal.persistence.NODE_DATABASE_PREFIX
 import net.corda.nodeapi.internal.persistence.currentDBSession
 import net.corda.nodeapi.internal.withContractsInJar
 import org.hibernate.query.Query
+import java.io.ByteArrayInputStream
 import java.io.FilterInputStream
 import java.io.IOException
 import java.io.InputStream
@@ -367,6 +369,9 @@ class NodeAttachmentService @JvmOverloads constructor(
                 // set the hash field of the new attachment record.
 
                 val bytes = inputStream.readFully()
+                require(!ZipBombDetector.scanZip(ByteArrayInputStream(bytes), servicesForResolution.networkParameters.maxTransactionSize.toLong())) {
+                    "The attachment is too large and exceeds both max transaction size and the maximum allowed compression ratio"
+                }
                 val id = bytes.sha256()
                 if (!hasAttachment(id)) {
                     checkIsAValidJAR(bytes.inputStream())