Skip to content

Commit

Permalink
Merge pull request #835 from ScorexFoundation/v5.0.1-tuple-fix
Browse files Browse the repository at this point in the history
Release Candidate v5.0.1
  • Loading branch information
aslesarenko authored Oct 25, 2022
2 parents 79c6f6f + ad40a6e commit 627e506
Show file tree
Hide file tree
Showing 16 changed files with 194 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -188,13 +188,20 @@ object ErgoBoxCandidate {
val value = r.getULong() // READ
val tree = DefaultSerializer.deserializeErgoTree(r, SigmaSerializer.MaxPropositionSize) // READ
val creationHeight = r.getUIntExact // READ
// NO-FORK: ^ in v5.x getUIntExact may throw Int overflow exception
// in v4.x r.getUInt().toInt is used and may return negative Int instead of the overflow
// and ErgoBoxCandidate with negative creation height is created, which is then invalidated
// during transaction validation. See validation rule # 122 in the Ergo node (ValidationRules.scala)
val nTokens = r.getUByte() // READ
val tokenIds = safeNewArray[Array[Byte]](nTokens)
val tokenAmounts = safeNewArray[Long](nTokens)
if (digestsInTx != null) {
val nDigests = digestsInTx.length
cfor(0)(_ < nTokens, _ + 1) { i =>
val digestIndex = r.getUIntExact // READ
// NO-FORK: in v5.x getUIntExact throws Int overflow exception
// in v4.x r.getUInt().toInt is used and may return negative Int in which case
// the error below is thrown
if (digestIndex < 0 || digestIndex >= nDigests)
sys.error(s"failed to find token id with index $digestIndex")
val amount = r.getULong() // READ
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@ object ErgoLikeTransactionSerializer extends SigmaSerializer[ErgoLikeTransaction

// parse distinct ids of tokens in transaction outputs
val tokensCount = r.getUIntExact
// NO-FORK: in v5.x getUIntExact may throw Int overflow exception
// in v4.x r.getUInt().toInt is used and may return negative Int instead of the overflow
// in which case the array allocation will throw NegativeArraySizeException
val tokens = safeNewArray[Array[Byte]](tokensCount)
cfor(0)(_ < tokensCount, _ + 1) { i =>
tokens(i) = r.getBytes(TokenId.size)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import sigmastate.serialization.SigmaSerializer
import sigmastate.utils.{SigmaByteReader, SigmaByteWriter}
import scalan.util.Extensions.{IntOps,LongOps}

// TODO v5.x: remove unused class and related json encoders
/** The rules are serialized ordered by ruleId.
* This serializer preserves roundtrip identity `deserialize(serialize(_)) = identity`
* however it may not preserve `serialize(deserialize(_)) = identity` */
Expand All @@ -19,7 +20,8 @@ object SigmaValidationSettingsSerializer extends SigmaSerializer[SigmaValidation
}

override def parse(r: SigmaByteReader): SigmaValidationSettings = {
val nRules = r.getUIntExact
val nRules = r.getUInt().toInt
// Note, when nRules < 0 as a result of Int overflow, the loop is empty
val parsed = (0 until nRules).map { _ =>
val ruleId = r.getUShort().toShortExact
val status = RuleStatusSerializer.parse(r)
Expand Down
7 changes: 5 additions & 2 deletions sigmastate/src/main/scala/sigmastate/AvlTreeData.scala
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,11 @@ object AvlTreeData {
override def parse(r: SigmaByteReader): AvlTreeData = {
val digest = r.getBytes(DigestSize)
val tf = AvlTreeFlags(r.getByte())
val keyLength = r.getUIntExact
val valueLengthOpt = r.getOption(r.getUIntExact)
val keyLength = r.getUInt().toInt
val valueLengthOpt = r.getOption(r.getUInt().toInt)
// Note, when keyLength and valueLengthOpt < 0 as a result of Int overflow,
// the deserializer succeeds with invalid AvlTreeData
// but still some AvlTree operations (remove_eval, update_eval, contains_eval) won't throw
AvlTreeData(ADDigest @@ digest, tf, keyLength, valueLengthOpt)
}
}
Expand Down
13 changes: 10 additions & 3 deletions sigmastate/src/main/scala/sigmastate/Values.scala
Original file line number Diff line number Diff line change
Expand Up @@ -890,19 +890,26 @@ object Values {
def tpe = SBox
}

// TODO refactor: only Constant make sense to inherit from EvaluatedValue

/** ErgoTree node which converts a collection of expressions into a tuple of data values
* of different types. Each data value of the resulting collection is obtained by
* evaluating the corresponding expression in `items`. All items may have different
* types.
*
* @param items source collection of expressions
*/
case class Tuple(items: IndexedSeq[Value[SType]]) extends Value[STuple] {
case class Tuple(items: IndexedSeq[Value[SType]])
extends EvaluatedValue[STuple] // note, this superclass is required as Tuple can be in a register
with EvaluatedCollection[SAny.type, STuple] {
override def companion = Tuple
override lazy val tpe = STuple(items.map(_.tpe))
override def opType: SFunc = ???
override def elementType: SAny.type = SAny

override lazy val value = {
val xs = items.cast[EvaluatedValue[SAny.type]].map(_.value)
Colls.fromArray(xs.toArray(SAny.classTag.asInstanceOf[ClassTag[SAny.WrappedType]]))(RType.AnyType)
}

protected final override def eval(env: DataEnv)(implicit E: ErgoTreeEvaluator): Any = {
// in v5.0 version we support only tuples of 2 elements to be equivalent with v4.x
if (items.length != 2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,12 @@ case class BlockValueSerializer(cons: (IndexedSeq[BlockItem], Value[SType]) => V

override def parse(r: SigmaByteReader): Value[SType] = {
val itemsSize = r.getUIntExact
val values: IndexedSeq[BlockItem] = if (itemsSize == 0)
// NO-FORK: in v5.x getUIntExact may throw Int overflow exception
// in v4.x r.getUInt().toInt is used and may return negative Int instead of the overflow
// in which case the array allocation will throw NegativeArraySizeException
val values: IndexedSeq[BlockItem] = if (itemsSize == 0) {
BlockItem.EmptySeq
}
else {
// HOTSPOT:: allocate new array only if it is not empty
val buf = safeNewArray[BlockItem](itemsSize)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ case class ConstantPlaceholderSerializer(cons: (Int, SType) => Value[SType])

override def parse(r: SigmaByteReader): Value[SType] = {
val id = r.getUIntExact
// NO-FORK: in v5.x getUIntExact may throw Int overflow exception
// in v4.x r.getUInt().toInt is used and may return negative Int instead of the overflow
// in which case the constantStore.get will throw ArrayIndexOutOfBoundsException
val constant = r.constantStore.get(id)
if (r.resolvePlaceholdersToConstants)
constant
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ object DataSerializer {
case SLong => r.getLong()
case SString =>
val size = r.getUIntExact
// NO-FORK: in v5.x getUIntExact may throw Int overflow exception
// in v4.x r.getUInt().toInt is used and may return negative Int instead of the overflow
// in which case the getBytes will throw NegativeArraySizeException
val bytes = r.getBytes(size)
new String(bytes, StandardCharsets.UTF_8)
case SBigInt =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,20 @@ class ErgoTreeSerializer {
val header = r.getByte()
CheckHeaderSizeBit(header)
val sizeOpt = if (ErgoTree.hasSize(header)) {
Some(r.getUIntExact)
val size = r.getUInt().toInt
// Note, when size < 0 as a result of Int overflow nothing happens here and later
// when deserialization proceeds normally as sizeOpt is not used on this pass.
// However, when ValidationException is thrown in deserializeErgoTree this negative
// tree size value will be used in
// val val numBytes = bodyPos - startPos + treeSize
// r.position = startPos
// val bytes = r.getBytes(numBytes) = bodyPos - startPos + treeSize
// val bytes = r.getBytes(numBytes)
// If numBytes < 0 then it throws on getBytes and the whole deserialization fails
// On the other hand if numBytes >= 0 then UnparsedErgoTree will be created.
// The Reader however will be in some unpredictable state, as not all ErgoTree bytes
// are consumed.
Some(size)
} else
None
(header, sizeOpt)
Expand All @@ -221,7 +234,10 @@ class ErgoTreeSerializer {
private def deserializeConstants(header: Byte, r: SigmaByteReader): IndexedSeq[Constant[SType]] = {
val constants: IndexedSeq[Constant[SType]] =
if (ErgoTree.isConstantSegregation(header)) {
val nConsts = r.getUIntExact
val nConsts = r.getUInt().toInt
// Note, when nConsts < 0 as a result of Int overflow, the empty seq is returned
// deserialization will succeed

if (nConsts > 0) {
// HOTSPOT:: allocate new array only if it is not empty
val res = safeNewArray[Constant[SType]](nConsts)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,15 @@ case class FuncValueSerializer(cons: (IndexedSeq[(Int, SType)], Value[SType]) =>

override def parse(r: SigmaByteReader): Value[SType] = {
val argsSize = r.getUIntExact
// NO-FORK: in v5.x getUIntExact may throw Int overflow exception
// in v4.x r.getUInt().toInt is used and may return negative Int instead of the overflow
// in which case the array allocation will throw NegativeArraySizeException
val args = safeNewArray[(Int, SType)](argsSize)
cfor(0)(_ < argsSize, _ + 1) { i =>
val id = r.getUIntExact
val id = r.getUInt().toInt
// Note, when id < 0 as a result of Int overflow, the r.valDefTypeStore(id) won't throw
// More over evaluation of such FuncValue will not throw either if the body contains
// ValUse with the same negative id
val tpe = r.getType()
r.valDefTypeStore(id) = tpe
args(i) = (id, tpe)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ case class ValDefSerializer(override val opDesc: ValueCompanion) extends ValueSe

override def parse(r: SigmaByteReader): Value[SType] = {
val id = r.getUIntExact
// NO-FORK: in v5.x getUIntExact may throw Int overflow exception
// in v4.x r.getUInt().toInt is used and may return negative Int instead of the overflow
// When id < 0 as a result of Int overflow, the r.valDefTypeStore(id) won't throw
// but ValDef constructor fails on require(id >= 0, "id must be >= 0")
val tpeArgs: Seq[STypeVar] = opCode match {
case FunDefCode =>
val nTpeArgs = r.getByte()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@ case class ValUseSerializer(cons: (Int, SType) => Value[SType]) extends ValueSer
}

override def parse(r: SigmaByteReader): Value[SType] = {
val id = r.getUIntExact
val id = r.getUInt.toInt
// Note, when id < 0 as a result of Int overflow, the r.valDefTypeStore(id) won't throw
// and also ValUse node will be created, but then its evaluation will throw (because
// there will be no ValDef with negative id in the env.
// However, in general, there is no guarantee that this ValUse will ever be executed
// as it may be in an `if` branch.
val tpe = r.valDefTypeStore(id)
cons(id, tpe)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ case class SigmaTransformerSerializer[I <: SigmaPropValue, O <: SigmaPropValue]

override def parse(r: SigmaByteReader): SigmaPropValue = {
val itemsSize = r.getUIntExact
// NO-FORK: in v5.x getUIntExact may throw Int overflow exception
// in v4.x r.getUInt().toInt is used and may return negative Int instead of the overflow
// in which case the array allocation will throw NegativeArraySizeException
val res = safeNewArray[SigmaPropValue](itemsSize)
cfor(0)(_ < itemsSize, _ + 1) { i =>
res(i) = r.getValue().asInstanceOf[SigmaPropValue]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ package org.ergoplatform
import org.ergoplatform.ErgoBox.TokenId
import org.ergoplatform.settings.ErgoAlgos
import scorex.crypto.hash.Digest32
import scorex.util.encode.Base16
import scorex.util.{Random, ModifierId}
import sigmastate.SCollection.SByteArray
import sigmastate.{SSigmaProp, SPair, SInt, TrivialProp, SType}
import sigmastate.{SSigmaProp, TrivialProp, SType, SPair, SInt}
import sigmastate.Values._
import sigmastate.interpreter.{ProverResult, ContextExtension}
import sigmastate.interpreter.{ContextExtension, ProverResult}
import sigmastate.serialization.SigmaSerializer
import sigmastate.eval._
import sigmastate.eval.Extensions._
Expand Down Expand Up @@ -300,4 +301,12 @@ class ErgoLikeTransactionSpec extends SigmaDslTesting {
}
}
}

property("Tuple in register test vector") {
val txId = "b5fd96c9f8c1ff436d609a225a12377c6873b890a145fc05f4099845b89315e5"
val txHex = "0278c93ee55eec066ec06290524dc01773440fbaff644bd181ab4334f97083863738644bf834de150b9abd077cf52ecb4892536ab6ad6ef8505fb932d9ca7b9fb211fc8294618723eb4909d4553f4d419762332f68865b1d703d00faae81c0e57683108c55647c5837a4fedfb52ffde04db74a8b4b77e37ab81afc3828c904862db6134e0f859ff5f62e4a5cb9da6b15ea1cd1a11dc932899d7b99e9073034ccd4e7c54fa9790d8ee356dce22ee151b044561c96000000038094ebdc031007040205c80105c8010500040004000e1f3faf2cb329f2e90d6d23b58d91bbb6c046aa143261cc21f52fbe2824bfcbf0d807d601e4c6a70408d602b2a5730000d603e4c6a70601d604e4c6a70856d605e4c6a70505d606e4c6a70705d60795720399c1a7c1720299c17202c1a7eb027201d1ededededededededed93c27202c2a793e4c672020408720193e4c6720205059572039d9c72057e8c7204010573019d9c72057e8c72040205730294e4c672020601720393e4c672020705720693e4c672020856720493e4c67202090ec5a7929c720672057207917207730395ef720393b1db630872027304d801d608b2db63087202730500ed938c7208017306938c7208027206d18f34000508cd030c8f9c4dc08f3c006fa85a47c9156dedbede000a8b764c6e374fd097e873ba0405dceeaa0401010564860202660263c0edcea7090008cd030c8f9c4dc08f3c006fa85a47c9156dedbede000a8b764c6e374fd097e873ba04d18f340000c0843d1005040004000e36100204a00b08cd0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798ea02d192a39a8cc7a701730073011001020402d19683030193a38cc7b2a57300000193c2b2a57301007473027303830108cdeeac93b1a57304d18f340000"
val txBytes = Base16.decode(txHex).get
val tx = ErgoLikeTransactionSerializer.fromBytes(txBytes)
tx.id shouldBe txId
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ trait NegativeTesting extends Matchers {
*/
def exceptionLike[E <: Throwable : ClassTag]
(msgParts: String*): Throwable => Boolean = {
case t: E => msgParts.forall(t.getMessage.contains(_))
case t: E => msgParts.forall(part => t.getMessage != null && t.getMessage.contains(part))
case _ => false
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,25 @@ import org.ergoplatform.validation.ValidationRules.CheckPositionLimit
import org.ergoplatform.{ErgoBoxCandidate, Outputs}
import org.scalacheck.Gen
import scalan.util.BenchmarkUtil
import scorex.crypto.authds.{ADKey, ADValue}
import scorex.crypto.authds.avltree.batch.{BatchAVLProver, Insert}
import scorex.crypto.hash.{Blake2b256, Digest32}
import scorex.util.serialization.{Reader, VLQByteBufferReader}
import sigmastate.Values.{BlockValue, GetVarInt, IntConstant, SValue, SigmaBoolean, SigmaPropValue, Tuple, ValDef, ValUse}
import sigmastate.Values.{SValue, BlockValue, GetVarInt, SigmaBoolean, ValDef, ValUse, SigmaPropValue, Tuple, IntConstant}
import sigmastate._
import sigmastate.eval.Extensions._
import sigmastate.eval._
import sigmastate.helpers.{ErgoLikeContextTesting, ErgoLikeTestInterpreter, SigmaTestingCommons}
import sigmastate.interpreter.{ContextExtension, CostedProverResult, CryptoConstants}
import sigmastate.lang.exceptions.{DeserializeCallDepthExceeded, InvalidTypePrefix, ReaderPositionLimitExceeded, SerializerException}
import sigmastate.helpers.{ErgoLikeTestInterpreter, SigmaTestingCommons, ErgoLikeContextTesting}
import sigmastate.interpreter.{CryptoConstants, CostedProverResult, ContextExtension}
import sigmastate.lang.exceptions.{ReaderPositionLimitExceeded, InvalidTypePrefix, DeserializeCallDepthExceeded, SerializerException}
import sigmastate.serialization.OpCodes._
import sigmastate.util.safeNewArray
import sigmastate.utils.SigmaByteReader
import sigmastate.utxo.SizeOf
import sigmastate.utils.Helpers._

import scala.collection.mutable
import scala.util.{Try, Success, Failure}

class DeserializationResilience extends SerializationSpecification
with SigmaTestingCommons with CrossVersionProps {
Expand Down Expand Up @@ -308,5 +313,101 @@ class DeserializationResilience extends SerializationSpecification
})
}

/** Because this method is called from many places it should always be called with `hint`. */
protected def checkResult[B](res: Try[B], expectedRes: Try[B], hint: String): Unit = {
(res, expectedRes) match {
case (Failure(exception), Failure(expectedException)) =>
withClue(hint) {
rootCause(exception).getClass shouldBe rootCause(expectedException).getClass
}
case _ =>
val actual = rootCause(res)
if (actual != expectedRes) {
assert(false, s"$hint\nActual: $actual;\nExpected: $expectedRes\n")
}
}
}

def writeUInt(x: Long): Array[Byte] = {
val w = SigmaSerializer.startWriter()
val bytes = w.putUInt(x).toBytes
bytes
}

def readToInt(bytes: Array[Byte]): Try[Int] = Try {
val r = SigmaSerializer.startReader(bytes)
r.getUInt().toInt
}

def readToIntExact(bytes: Array[Byte]): Try[Int] = Try {
val r = SigmaSerializer.startReader(bytes)
r.getUIntExact
}

property("getUIntExact vs getUInt().toInt") {
val intOverflow = Failure(new ArithmeticException("Int overflow"))
val cases = Table(("stored", "toInt", "toIntExact"),
(0L, Success(0), Success(0)),
(Int.MaxValue.toLong - 1, Success(Int.MaxValue - 1), Success(Int.MaxValue - 1)),
(Int.MaxValue.toLong, Success(Int.MaxValue), Success(Int.MaxValue)),
(Int.MaxValue.toLong + 1, Success(Int.MinValue), intOverflow),
(Int.MaxValue.toLong + 2, Success(Int.MinValue + 1), intOverflow),
(0xFFFFFFFFL, Success(-1), intOverflow)
)
forAll(cases) { (x, res, resExact) =>
val bytes = writeUInt(x)
checkResult(readToInt(bytes), res, "toInt")
checkResult(readToIntExact(bytes), resExact, "toIntExact")
}

// check it is impossible to write negative value with reference implementation of serializer
// ALSO NOTE that VLQ encoded bytes are always interpreted as positive Long
// so the difference between getUIntExact vs getUInt().toInt boils down to how Int.MaxValue
// overflow is handled
assertExceptionThrown(
writeUInt(-1L),
exceptionLike[IllegalArgumentException]("-1 is out of unsigned int range")
)

val MaxUIntPlusOne = 0xFFFFFFFFL + 1
assertExceptionThrown(
writeUInt(MaxUIntPlusOne),
exceptionLike[IllegalArgumentException](s"$MaxUIntPlusOne is out of unsigned int range")
)
}

property("test assumptions of how negative value from getUInt().toInt is handled") {
assertExceptionThrown(
safeNewArray[Int](-1),
exceptionLike[NegativeArraySizeException]())

val bytes = writeUInt(10)
val store = new ConstantStore(IndexedSeq(IntConstant(1)))

val r = SigmaSerializer.startReader(bytes, store, true)
assertExceptionThrown(
r.constantStore.get(-1),
exceptionLike[ArrayIndexOutOfBoundsException]())

assertExceptionThrown(
r.getBytes(-1),
exceptionLike[NegativeArraySizeException]())

r.valDefTypeStore(-1) = SInt // no exception on negative key

// the following example shows how far negative keyLength can go inside AvlTree operations
val avlProver = new BatchAVLProver[Digest32, Blake2b256.type](keyLength = 32, None)
val digest = avlProver.digest
val flags = AvlTreeFlags(true, false, false)
val treeData = new AvlTreeData(digest, flags, -1, None)
val tree = SigmaDsl.avlTree(treeData)
val k = Blake2b256.hash("1")
val v = k
avlProver.performOneOperation(Insert(ADKey @@ k, ADValue @@ v))
val proof = avlProver.generateProof()
val verifier = tree.createVerifier(Colls.fromArray(proof))
verifier.performOneOperation(Insert(ADKey @@ k, ADValue @@ v)).isFailure shouldBe true
// NOTE, even though performOneOperation fails, some AvlTree$ methods used in Interpreter
// (remove_eval, update_eval, contains_eval) won't throw, while others will.
}
}

0 comments on commit 627e506

Please sign in to comment.