Merge pull request #64 from input-output-hk/manifest-ser

Manifest (de-)serialization fix

Author: kushti

README.md

@@ -18,7 +18,7 @@ resolvers += "Sonatype Releases" at "https://oss.sonatype.org/content/repositori
 
 You can use Scrypto in your sbt project by simply adding the following dependency to your build file:
 ```scala
-libraryDependencies += "org.scorexfoundation" %% "scrypto" % "2.1.5"
+libraryDependencies += "org.scorexfoundation" %% "scrypto" % "2.1.6"
 ```
 
 ### Hash functions

build.sbt

@@ -4,8 +4,8 @@ name := "scrypto"
 
 lazy val commonSettings = Seq(
   organization := "org.scorexfoundation",
-  version := "2.1.5",
-  scalaVersion := "2.12.7",
+  version := "2.1.6",
+  scalaVersion := "2.12.8",
   resolvers += Resolver.sonatypeRepo("public"),
   licenses := Seq("CC0" -> url("https://creativecommons.org/publicdomain/zero/1.0/legalcode")),
   homepage := Some(url("https://github.com/input-output-hk/scrypto")),

lock.sbt

@@ -6,12 +6,12 @@ dependencyOverrides in ThisBuild ++= Seq(
   "ch.qos.logback" % "logback-core" % "1.3.0-alpha4",
   "com.google.guava" % "guava" % "21.0",
   "com.sun.mail" % "javax.mail" % "1.6.0",
-  "com.typesafe.scala-logging" % "scala-logging_2.12" % "3.9.0",
+  "com.typesafe.scala-logging" % "scala-logging_2.12" % "3.9.2",
   "javax.activation" % "activation" % "1.1",
   "org.bouncycastle" % "bcprov-jdk15on" % "1.60",
   "org.rudogma" % "supertagged_2.12" % "1.4",
   "org.scorexfoundation" % "scorex-util_2.12" % "0.1.1",
   "org.slf4j" % "slf4j-api" % "1.8.0-beta1",
   "org.whispersystems" % "curve25519-java" % "0.5.0"
 )
-// LIBRARY_DEPENDENCIES_HASH 5bccd31374d5a2878cfcd4fc35339324fc38aea3
+// LIBRARY_DEPENDENCIES_HASH 09aa9b478d26dc10e237ea6c0f62986425561937

release-notes.md

@@ -1,3 +1,9 @@
+**2.1.6**
+---------
+
+* Manifest deserialization now checks that valueLength is not negative
+
+
 **2.1.5**
 ---------
 

src/main/scala/scorex/crypto/authds/avltree/batch/BatchAVLProver.scala

@@ -8,7 +8,6 @@ import scorex.utils.ByteArray
 
 import scala.annotation.tailrec
 import scala.collection.mutable
-import scala.collection.mutable.ArrayBuffer
 import scala.util.{Failure, Random, Success, Try}
 
 

src/main/scala/scorex/crypto/authds/avltree/batch/serialization/BatchAVLProverManifest.scala

@@ -9,4 +9,4 @@ import scorex.crypto.hash.{CryptographicHash, Digest}
   */
 case class BatchAVLProverManifest[D <: Digest, HF <: CryptographicHash[D]](keyLength: Int,
                                                                            valueLengthOpt: Option[Int],
-                                                                           oldRootAndHeight: (ProverNodes[D], Int))
+                                                                           rootAndHeight: (ProverNodes[D], Int))

src/main/scala/scorex/crypto/authds/avltree/batch/serialization/BatchAVLProverSerializer.scala

@@ -55,7 +55,7 @@ class BatchAVLProverSerializer[D <: Digest, HF <: CryptographicHash[D]](implicit
     * Combine tree pieces into one big tree
     */
   def combine(sliced: SlicedTree): Try[BatchAVLProver[D, HF]] = Try {
-    sliced._1.oldRootAndHeight._1 match {
+    sliced._1.rootAndHeight._1 match {
       case tn: InternalProverNode[D] =>
         def mutateLoop(n: ProverNodes[D]): Unit = n match {
           case n: ProxyInternalNode[D] if n.isEmpty =>
@@ -70,26 +70,27 @@ class BatchAVLProverSerializer[D <: Digest, HF <: CryptographicHash[D]](implicit
         }
 
         mutateLoop(tn)
-        new BatchAVLProver[D, HF](sliced._1.keyLength, sliced._1.valueLengthOpt, Some(sliced._1.oldRootAndHeight))
-      case l: ProverLeaf[D] =>
-        new BatchAVLProver[D, HF](sliced._1.keyLength, sliced._1.valueLengthOpt, Some(sliced._1.oldRootAndHeight))
+        new BatchAVLProver[D, HF](sliced._1.keyLength, sliced._1.valueLengthOpt, Some(sliced._1.rootAndHeight))
+      case _: ProverLeaf[D] =>
+        new BatchAVLProver[D, HF](sliced._1.keyLength, sliced._1.valueLengthOpt, Some(sliced._1.rootAndHeight))
     }
   }
 
-  def manifestToBytes(m: BatchAVLProverManifest[D, HF]): Array[Byte] = {
-    Bytes.concat(Ints.toByteArray(m.keyLength),
-      Ints.toByteArray(m.valueLengthOpt.getOrElse(-1)),
-      Ints.toByteArray(m.oldRootAndHeight._2),
-      nodesToBytes(m.oldRootAndHeight._1)
+  def manifestToBytes(manifest: BatchAVLProverManifest[D, HF]): Array[Byte] = {
+    Bytes.concat(Ints.toByteArray(manifest.keyLength),
+      Ints.toByteArray(manifest.valueLengthOpt.getOrElse(-1)),
+      Ints.toByteArray(manifest.rootAndHeight._2),
+      nodesToBytes(manifest.rootAndHeight._1)
     )
   }
 
-  def manifestFromBytes(b: Array[Byte]): Try[BatchAVLProverManifest[D, HF]] = Try {
-    val keyLength = Ints.fromByteArray(b.slice(0, 4))
-    val valueLength = Ints.fromByteArray(b.slice(4, 8))
+  def manifestFromBytes(bytes: Array[Byte]): Try[BatchAVLProverManifest[D, HF]] = Try {
+    val keyLength = Ints.fromByteArray(bytes.slice(0, 4))
+    val valueLength = Ints.fromByteArray(bytes.slice(4, 8))
+    if (valueLength < -1) throw new Error(s"Wrong valueLength: $valueLength")
     val valueLengthOpt = if (valueLength == -1) None else Some(valueLength)
-    val oldHeight = Ints.fromByteArray(b.slice(8, 12))
-    val oldTop = nodesFromBytes(b.slice(12, b.length), keyLength).get
+    val oldHeight = Ints.fromByteArray(bytes.slice(8, 12))
+    val oldTop = nodesFromBytes(bytes.slice(12, bytes.length), keyLength).get
     BatchAVLProverManifest[D, HF](keyLength, valueLengthOpt, (oldTop, oldHeight))
   }
 
@@ -113,7 +114,7 @@ class BatchAVLProverSerializer[D <: Digest, HF <: CryptographicHash[D]](implicit
     loop(obj)
   }
 
-  def nodesFromBytes(bytesIN: Array[Byte], keyLength: Int): Try[ProverNodes[D]] = Try {
+  def nodesFromBytes(bytesIn: Array[Byte], keyLength: Int): Try[ProverNodes[D]] = Try {
     def loop(bytes: Array[Byte]): ProverNodes[D] = bytes.head match {
       case 0 =>
         val key = ADKey @@ bytes.slice(1, keyLength + 1)
@@ -140,7 +141,7 @@ class BatchAVLProverSerializer[D <: Digest, HF <: CryptographicHash[D]](implicit
         ???
     }
 
-    loop(bytesIN)
+    loop(bytesIn)
   }
 }
 

src/main/scala/scorex/crypto/authds/avltree/batch/serialization/BatchAVLProverSubtree.scala

@@ -1,15 +1,9 @@
 package scorex.crypto.authds.avltree.batch.serialization
 
-import scorex.crypto.authds.avltree.batch.{InternalProverNode, ProverLeaf, ProverNodes}
+import scorex.crypto.authds.avltree.batch.ProverNodes
 import scorex.crypto.hash.{CryptographicHash, Digest}
 
-import scala.util.Try
-
 /**
   * AVL subtree, starting from Manifests FinalInternalNode and ending with Leafs
   */
-case class BatchAVLProverSubtree[D <: Digest, HF <: CryptographicHash[D]](subtreeTop: ProverNodes[D])
-
-object BatchAVLProverSubtreeSerializer {
-
-}
+case class BatchAVLProverSubtree[D <: Digest, HF <: CryptographicHash[D]](subtreeTop: ProverNodes[D])

src/test/scala/scorex/crypto/authds/avltree/batch/serialization/AVLBatchSerializationSpecification.scala

@@ -27,23 +27,23 @@ class AVLBatchSerializationSpecification extends PropSpec with GeneratorDrivenPr
   private def generateProver(size: Int = InitialTreeSize): BatchAVLProver[D, HF] = {
     val prover = new BatchAVLProver[D, HF](KL, None)
     val keyValues = (0 until size) map { i =>
-      (ADKey @@ Blake2b256(i.toString.getBytes("UTF-8")).take(KL), ADValue @@ (i.toString.getBytes("UTF-8")))
+      (ADKey @@ Blake2b256(i.toString.getBytes("UTF-8")).take(KL), ADValue @@ i.toString.getBytes("UTF-8"))
     }
     keyValues.foreach(kv => prover.performOneOperation(Insert(kv._1, kv._2)))
     prover.generateProof()
     prover
   }
 
   property("slice to pieces and combine tree back") {
-    forAll(Gen.choose(100, 100000)) { treeSize: Int =>
-      whenever(treeSize >= 100) {
+    forAll(Gen.choose(10, 100000)) { treeSize: Int =>
+      whenever(treeSize >= 10) {
         val tree = generateProver(treeSize)
         val height = tree.rootNodeHeight
         val digest = tree.digest
         val serializer = new BatchAVLProverSerializer[D, HF]
         val sliced = serializer.slice(tree)
 
-        val manifestLeftTree = leftTree(sliced._1.oldRootAndHeight._1)
+        val manifestLeftTree = leftTree(sliced._1.rootAndHeight._1)
         val subtreeLeftTree = leftTree(sliced._2.head.subtreeTop)
 
         manifestLeftTree.length should be < height
@@ -56,15 +56,14 @@ class AVLBatchSerializationSpecification extends PropSpec with GeneratorDrivenPr
   }
 
   property("slice to Array[Byte] pieces and combine tree back") {
-    forAll(Gen.choose(100, 100000)) { treeSize: Int =>
+    forAll(Gen.choose(0, 100000)) { treeSize: Int =>
       val serializer = new BatchAVLProverSerializer[D, HF]
       val tree = generateProver(treeSize)
       val kl = tree.keyLength
       val digest = tree.digest
 
       val sliced = serializer.slice(tree)
 
-
       val manifestBytes = serializer.manifestToBytes(sliced._1)
       val subtreeBytes = sliced._2.map(t => serializer.subtreeToBytes(t))
 
@@ -83,7 +82,7 @@ class AVLBatchSerializationSpecification extends PropSpec with GeneratorDrivenPr
 
   property("manifest serialization") {
     val serializer = new BatchAVLProverSerializer[D, HF]
-    forAll(Gen.choose(100, 100000)) { treeSize: Int =>
+    forAll(Gen.choose(0, 100000)) { treeSize: Int =>
       val tree = generateProver(treeSize)
       val kl = tree.keyLength
       val digest = tree.digest
@@ -93,10 +92,20 @@ class AVLBatchSerializationSpecification extends PropSpec with GeneratorDrivenPr
       val manifestBytes = serializer.manifestToBytes(manifest)
       val deserializedManifest = serializer.manifestFromBytes(manifestBytes).get
 
-      deserializedManifest.oldRootAndHeight._1.label shouldBe manifest.oldRootAndHeight._1.label
+      deserializedManifest.rootAndHeight._1.label shouldBe manifest.rootAndHeight._1.label
     }
   }
 
+  property("wrong manifest") {
+    val tree = generateProver()
+    val serializer = new BatchAVLProverSerializer[D, HF]
+    val sliced = serializer.slice(tree)
+    val wrongManifest: BatchAVLProverManifest[D, HF] = sliced._1.copy(valueLengthOpt = Some(-2))
+
+    val manifestBytes = serializer.manifestToBytes(wrongManifest)
+    serializer.manifestFromBytes(manifestBytes).isFailure shouldBe true
+  }
+
   def leftTree(n: ProverNodes[D]): Seq[ProverNodes[D]] = n match {
     case n: ProxyInternalNode[D] if n.isEmpty =>
       Seq(n)