Fix bounds checking for MultiSignatureScriptPubKey.maxSigsRequired (#5365)

This commit is contained in:
Chris Stewart 2024-01-21 09:17:24 -06:00 committed by GitHub
parent 83a27e2775
commit 0f95a1f7bb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 43 additions and 12 deletions

View file

@ -412,6 +412,13 @@ class TransactionTest extends BitcoinSUnitTest {
assert(tx.hex == hex)
}
it must "parse decb09c41bc76bad8e006d92cf9f8c7ad4114e441a9cc6daf149e1495593a82f" in {
val hex =
"01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff5303de1304040007762f44124d696e656420627920425443204775696c642cfabe6d6d4da12b2799cc8b6bba921e104cc9054fd1da40ed6c1b6287efd8460475f2a0440100000000000000080247961b56ae0000ffffffff018759b396000000001976a91427a1f12771de5cc3b73941664b2537c15316be4388ac00000000"
val tx = Transaction.fromHex(hex)
assert(tx.hex == hex)
}
private def findInput(
tx: Transaction,
outPoint: TransactionOutPoint): Option[(TransactionInput, Int)] = {

View file

@ -118,9 +118,16 @@ sealed trait MultiSignatureScriptPubKey extends RawScriptPubKey {
val numSigsRequired = asmWithoutPushOps(opCheckMultiSigIndex - maxSigs - 2)
numSigsRequired match {
case x: ScriptNumber => x.toInt
case c: ScriptConstant
if ScriptNumber(c.hex).toLong <= Consensus.maxPublicKeysPerMultiSig =>
ScriptNumber(c.hex).toInt
case c: ScriptConstant =>
val sn = ScriptNumber(c.bytes).toInt
val inBounds =
sn >= 0 && sn <= Consensus.maxPublicKeysPerMultiSig
if (inBounds) {
sn
} else {
sys.error(
s"Negative numSignaturesRequired given for MultiSignatureSPK, got=$sn")
}
case _ =>
throw new RuntimeException(
"The first element of the multisignature pubkey must be a script number operation\n" +
@ -137,10 +144,16 @@ sealed trait MultiSignatureScriptPubKey extends RawScriptPubKey {
} else {
asm(checkMultiSigIndex - 1) match {
case x: ScriptNumber => x.toInt
case c: ScriptConstant
if ScriptNumber(
c.hex).toLong <= Consensus.maxPublicKeysPerMultiSig =>
ScriptNumber(c.hex).toInt
case c: ScriptConstant =>
val maxSigs = ScriptNumber(c.bytes).toInt
val inBounds =
maxSigs >= 0 && maxSigs <= Consensus.maxPublicKeysPerMultiSig
if (inBounds) {
maxSigs
} else {
sys.error(
s"Negative maxSigs given for MultiSignatureSPK, got=$maxSigs")
}
case x =>
throw new RuntimeException(
"The element preceding a OP_CHECKMULTISIG operation in a multisignature pubkey must be a script number operation, got: " + x)
@ -225,8 +238,10 @@ object MultiSignatureScriptPubKey
/** Determines if the given script tokens are a multisignature `scriptPubKey` */
override def isValidAsm(asm: Seq[ScriptToken]): Boolean = {
val containsMultiSigOp =
asm.contains(OP_CHECKMULTISIG) || asm.contains(OP_CHECKMULTISIGVERIFY)
val cmsIdx = if (asm.contains(OP_CHECKMULTISIG)) {
asm.indexOf(OP_CHECKMULTISIG)
} else asm.indexOf(OP_CHECKMULTISIGVERIFY)
val containsMultiSigOp = cmsIdx != -1
if (asm.nonEmpty && containsMultiSigOp) {
//we need either the first or second asm operation to indicate how many signatures are required
@ -242,11 +257,20 @@ object MultiSignatureScriptPubKey
}
}
//the second to last asm operation should be the maximum amount of public keys
val hasMaximumSignaturesTry = Try {
asm(asm.length - 2) match {
val hasMaximumSignaturesTry = {
val maxSigsIdx = asm.length - 2
if (maxSigsIdx >= cmsIdx) {
val exn = new IllegalAccessException(
s"maxSigsIdx is after OP_CHECKMULTISIG/OP_CHECKMULTISIGVERIFY, maxSigsIx=$maxSigsIdx")
Failure(exn)
} else {
Try {
asm(maxSigsIdx) match {
case token: ScriptToken => isValidPubKeyNumber(token)
}
}
}
}
(hasRequiredSignaturesTry, hasMaximumSignaturesTry) match {
case (Success(hasRequiredSignatures), Success(hasMaximumSignatures)) =>