Skip to content

Commit 6259937

Browse files
gavinandresengavinandresen
committed
Merge pull request dashpay#4365 from gavinandresen/relax_isstandard
Relax IsStandard rules for pay-to-script-hash transactions
2 parents 4684c88 + 7f3b4e9 commit 6259937

File tree

3 files changed

+97
-66
lines changed

3 files changed

+97
-66
lines changed

‎src/main.cpp‎

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -583,15 +583,13 @@ bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime)
583583
}
584584

585585
//
586-
// Check transaction inputs, and make sure any
587-
// pay-to-script-hash transactions are evaluating IsStandard scripts
586+
// Check transaction inputs to mitigate two
587+
// potential denial-of-service attacks:
588588
//
589-
// Why bother? To avoid denial-of-service attacks; an attacker
590-
// can submit a standard HASH... OP_EQUAL transaction,
591-
// which will get accepted into blocks. The redemption
592-
// script can be anything; an attacker could use a very
593-
// expensive-to-check-upon-redemption script like:
594-
// DUP CHECKSIG DROP ... repeated 100 times... OP_1
589+
// 1. scriptSigs with extra data stuffed into them,
590+
// not consumed by scriptPubKey (or P2SH script)
591+
// 2. P2SH scripts with a crazy number of expensive
592+
// CHECKSIG/CHECKMULTISIG operations
595593
//
596594
bool AreInputsStandard(const CTransaction& tx, CCoinsViewCache& mapInputs)
597595
{
@@ -615,8 +613,9 @@ bool AreInputsStandard(const CTransaction& tx, CCoinsViewCache& mapInputs)
615613
// Transactions with extra stuff in their scriptSigs are
616614
// non-standard. Note that this EvalScript() call will
617615
// be quick, because if there are any operations
618-
// beside "push data" in the scriptSig the
619-
// IsStandard() call returns false
616+
// beside "push data" in the scriptSig
617+
// IsStandard() will have already returned false
618+
// and this method isn't called.
620619
vector<vector<unsigned char> > stack;
621620
if (!EvalScript(stack, tx.vin[i].scriptSig, tx, i, false, 0))
622621
return false;
@@ -628,16 +627,20 @@ bool AreInputsStandard(const CTransaction& tx, CCoinsViewCache& mapInputs)
628627
CScript subscript(stack.back().begin(), stack.back().end());
629628
vector<vector<unsigned char> > vSolutions2;
630629
txnouttype whichType2;
631-
if (!Solver(subscript, whichType2, vSolutions2))
632-
return false;
633-
if (whichType2 == TX_SCRIPTHASH)
634-
return false;
635-
636-
int tmpExpected;
637-
tmpExpected = ScriptSigArgsExpected(whichType2, vSolutions2);
638-
if (tmpExpected < 0)
639-
return false;
640-
nArgsExpected += tmpExpected;
630+
if (Solver(subscript, whichType2, vSolutions2))
631+
{
632+
int tmpExpected = ScriptSigArgsExpected(whichType2, vSolutions2);
633+
if (tmpExpected < 0)
634+
return false;
635+
nArgsExpected += tmpExpected;
636+
}
637+
else
638+
{
639+
// Any other Script with less than 15 sigops OK:
640+
unsigned int sigops = subscript.GetSigOpCount(true);
641+
// ... extra data left on the stack after execution is OK, too:
642+
return (sigops <= MAX_P2SH_SIGOPS);
643+
}
641644
}
642645

643646
if (stack.size() != (unsigned int)nArgsExpected)

‎src/main.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ static const unsigned int DEFAULT_BLOCK_PRIORITY_SIZE = 50000;
4343
static const unsigned int MAX_STANDARD_TX_SIZE = 100000;
4444
/** The maximum allowed number of signature check operations in a block (network rule) */
4545
static const unsigned int MAX_BLOCK_SIGOPS = MAX_BLOCK_SIZE/50;
46+
/** Maxiumum number of signature check operations in an IsStandard() P2SH script */
47+
static const unsigned int MAX_P2SH_SIGOPS = 15;
4648
/** The maximum number of orphan transactions kept in memory */
4749
static const unsigned int MAX_ORPHAN_TRANSACTIONS = MAX_BLOCK_SIZE/100;
4850
/** Default for -maxorphanblocks, maximum number of orphan blocks kept in memory */

‎src/test/script_P2SH_tests.cpp‎

Lines changed: 72 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -256,66 +256,86 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard)
256256
CCoinsView coinsDummy;
257257
CCoinsViewCache coins(coinsDummy);
258258
CBasicKeyStore keystore;
259-
CKey key[3];
259+
CKey key[6];
260260
vector<CPubKey> keys;
261-
for (int i = 0; i < 3; i++)
261+
for (int i = 0; i < 6; i++)
262262
{
263263
key[i].MakeNewKey(true);
264264
keystore.AddKey(key[i]);
265-
keys.push_back(key[i].GetPubKey());
266265
}
266+
for (int i = 0; i < 3; i++)
267+
keys.push_back(key[i].GetPubKey());
267268

268269
CMutableTransaction txFrom;
269-
txFrom.vout.resize(6);
270+
txFrom.vout.resize(7);
270271

271272
// First three are standard:
272273
CScript pay1; pay1.SetDestination(key[0].GetPubKey().GetID());
273274
keystore.AddCScript(pay1);
274-
CScript payScriptHash1; payScriptHash1.SetDestination(pay1.GetID());
275275
CScript pay1of3; pay1of3.SetMultisig(1, keys);
276276

277-
txFrom.vout[0].scriptPubKey = payScriptHash1;
277+
txFrom.vout[0].scriptPubKey.SetDestination(pay1.GetID()); // P2SH (OP_CHECKSIG)
278278
txFrom.vout[0].nValue = 1000;
279-
txFrom.vout[1].scriptPubKey = pay1;
279+
txFrom.vout[1].scriptPubKey = pay1; // ordinary OP_CHECKSIG
280280
txFrom.vout[1].nValue = 2000;
281-
txFrom.vout[2].scriptPubKey = pay1of3;
281+
txFrom.vout[2].scriptPubKey = pay1of3; // ordinary OP_CHECKMULTISIG
282282
txFrom.vout[2].nValue = 3000;
283283

284-
// Last three non-standard:
285-
CScript empty;
286-
keystore.AddCScript(empty);
287-
txFrom.vout[3].scriptPubKey = empty;
284+
// vout[3] is complicated 1-of-3 AND 2-of-3
285+
// ... that is OK if wrapped in P2SH:
286+
CScript oneAndTwo;
287+
oneAndTwo << OP_1 << key[0].GetPubKey() << key[1].GetPubKey() << key[2].GetPubKey();
288+
oneAndTwo << OP_3 << OP_CHECKMULTISIGVERIFY;
289+
oneAndTwo << OP_2 << key[3].GetPubKey() << key[4].GetPubKey() << key[5].GetPubKey();
290+
oneAndTwo << OP_3 << OP_CHECKMULTISIG;
291+
keystore.AddCScript(oneAndTwo);
292+
txFrom.vout[3].scriptPubKey.SetDestination(oneAndTwo.GetID());
288293
txFrom.vout[3].nValue = 4000;
289-
// Can't use SetPayToScriptHash, it checks for the empty Script. So:
290-
txFrom.vout[4].scriptPubKey << OP_HASH160 << Hash160(empty) << OP_EQUAL;
294+
295+
// vout[4] is max sigops:
296+
CScript fifteenSigops; fifteenSigops << OP_1;
297+
for (int i = 0; i < MAX_P2SH_SIGOPS; i++)
298+
fifteenSigops << key[i%3].GetPubKey();
299+
fifteenSigops << OP_15 << OP_CHECKMULTISIG;
300+
keystore.AddCScript(fifteenSigops);
301+
txFrom.vout[4].scriptPubKey.SetDestination(fifteenSigops.GetID());
291302
txFrom.vout[4].nValue = 5000;
292-
CScript oneOfEleven;
293-
oneOfEleven << OP_1;
294-
for (int i = 0; i < 11; i++)
295-
oneOfEleven << key[0].GetPubKey();
296-
oneOfEleven << OP_11 << OP_CHECKMULTISIG;
297-
txFrom.vout[5].scriptPubKey.SetDestination(oneOfEleven.GetID());
298-
txFrom.vout[5].nValue = 6000;
303+
304+
// vout[5/6] are non-standard because they exceed MAX_P2SH_SIGOPS
305+
CScript sixteenSigops; sixteenSigops << OP_16 << OP_CHECKMULTISIG;
306+
keystore.AddCScript(sixteenSigops);
307+
txFrom.vout[5].scriptPubKey.SetDestination(fifteenSigops.GetID());
308+
txFrom.vout[5].nValue = 5000;
309+
CScript twentySigops; twentySigops << OP_CHECKMULTISIG;
310+
keystore.AddCScript(twentySigops);
311+
txFrom.vout[6].scriptPubKey.SetDestination(twentySigops.GetID());
312+
txFrom.vout[6].nValue = 6000;
313+
299314

300315
coins.SetCoins(txFrom.GetHash(), CCoins(txFrom, 0));
301316

302317
CMutableTransaction txTo;
303318
txTo.vout.resize(1);
304319
txTo.vout[0].scriptPubKey.SetDestination(key[1].GetPubKey().GetID());
305320

306-
txTo.vin.resize(3);
307-
txTo.vin[0].prevout.n = 0;
308-
txTo.vin[0].prevout.hash = txFrom.GetHash();
321+
txTo.vin.resize(5);
322+
for (int i = 0; i < 5; i++)
323+
{
324+
txTo.vin[i].prevout.n = i;
325+
txTo.vin[i].prevout.hash = txFrom.GetHash();
326+
}
309327
BOOST_CHECK(SignSignature(keystore, txFrom, txTo, 0));
310-
txTo.vin[1].prevout.n = 1;
311-
txTo.vin[1].prevout.hash = txFrom.GetHash();
312328
BOOST_CHECK(SignSignature(keystore, txFrom, txTo, 1));
313-
txTo.vin[2].prevout.n = 2;
314-
txTo.vin[2].prevout.hash = txFrom.GetHash();
315329
BOOST_CHECK(SignSignature(keystore, txFrom, txTo, 2));
330+
// SignSignature doesn't know how to sign these. We're
331+
// not testing validating signatures, so just create
332+
// dummy signatures that DO include the correct P2SH scripts:
333+
txTo.vin[3].scriptSig << OP_11 << OP_11 << static_cast<vector<unsigned char> >(oneAndTwo);
334+
txTo.vin[4].scriptSig << static_cast<vector<unsigned char> >(fifteenSigops);
316335

317336
BOOST_CHECK(::AreInputsStandard(txTo, coins));
318-
BOOST_CHECK_EQUAL(GetP2SHSigOpCount(txTo, coins), 1U);
337+
// 22 P2SH sigops for all inputs (1 for vin[0], 6 for vin[3], 15 for vin[4]
338+
BOOST_CHECK_EQUAL(GetP2SHSigOpCount(txTo, coins), 22U);
319339

320340
// Make sure adding crap to the scriptSigs makes them non-standard:
321341
for (int i = 0; i < 3; i++)
@@ -326,23 +346,29 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard)
326346
txTo.vin[i].scriptSig = t;
327347
}
328348

329-
CMutableTransaction txToNonStd;
330-
txToNonStd.vout.resize(1);
331-
txToNonStd.vout[0].scriptPubKey.SetDestination(key[1].GetPubKey().GetID());
332-
txToNonStd.vout[0].nValue = 1000;
333-
txToNonStd.vin.resize(2);
334-
txToNonStd.vin[0].prevout.n = 4;
335-
txToNonStd.vin[0].prevout.hash = txFrom.GetHash();
336-
txToNonStd.vin[0].scriptSig << Serialize(empty);
337-
txToNonStd.vin[1].prevout.n = 5;
338-
txToNonStd.vin[1].prevout.hash = txFrom.GetHash();
339-
txToNonStd.vin[1].scriptSig << OP_0 << Serialize(oneOfEleven);
340-
341-
BOOST_CHECK(!::AreInputsStandard(txToNonStd, coins));
342-
BOOST_CHECK_EQUAL(GetP2SHSigOpCount(txToNonStd, coins), 11U);
343-
344-
txToNonStd.vin[0].scriptSig.clear();
345-
BOOST_CHECK(!::AreInputsStandard(txToNonStd, coins));
349+
CMutableTransaction txToNonStd1;
350+
txToNonStd1.vout.resize(1);
351+
txToNonStd1.vout[0].scriptPubKey.SetDestination(key[1].GetPubKey().GetID());
352+
txToNonStd1.vout[0].nValue = 1000;
353+
txToNonStd1.vin.resize(1);
354+
txToNonStd1.vin[0].prevout.n = 5;
355+
txToNonStd1.vin[0].prevout.hash = txFrom.GetHash();
356+
txToNonStd1.vin[0].scriptSig << static_cast<vector<unsigned char> >(sixteenSigops);
357+
358+
BOOST_CHECK(!::AreInputsStandard(txToNonStd1, coins));
359+
BOOST_CHECK_EQUAL(GetP2SHSigOpCount(txToNonStd1, coins), 16U);
360+
361+
CMutableTransaction txToNonStd2;
362+
txToNonStd2.vout.resize(1);
363+
txToNonStd2.vout[0].scriptPubKey.SetDestination(key[1].GetPubKey().GetID());
364+
txToNonStd2.vout[0].nValue = 1000;
365+
txToNonStd2.vin.resize(1);
366+
txToNonStd2.vin[0].prevout.n = 6;
367+
txToNonStd2.vin[0].prevout.hash = txFrom.GetHash();
368+
txToNonStd2.vin[0].scriptSig << static_cast<vector<unsigned char> >(twentySigops);
369+
370+
BOOST_CHECK(!::AreInputsStandard(txToNonStd2, coins));
371+
BOOST_CHECK_EQUAL(GetP2SHSigOpCount(txToNonStd2, coins), 20U);
346372
}
347373

348374
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)