Potential Lockscript Standardness DoS vector in Contract Dev

#1

Just want to spitball and create some discussion around a potential DoS vector that can affect some Smart Contracts. At the very least, I think it’s a point that’s worth noting (and accommodating) for contract devs as it’s not immediately obvious.

  1. Imagine a two-party contract where the second party joins the contract later and provides a payout address (as LockingBytecode) as DATA in some form (not as an output at this point).
  2. The contract is setup to settle later and the contract will have two outputs:
    a) To 1st party’s provided locking bytecode
    b) To 2nd party’s provided locking bytecode

Example just to demonstrate the concept:

contract ExampleContract (bytes partyALockingBytecode) {
    function joinContract(bytes partyBLockingBytecode) {
        // Require that the value of the commitment is B's Locking Bytecode.
        require(tx.outputs[0].nftCommitment == partyBLockingBytecode);

        // Require that it pays back to itself (for simplicity).
        require(tx.outputs[0].lockingBytecode == tx.inputs[0].lockingBytecode);
    }

    function payout() {
        // Get the amount available in contract.
        int amountAvailableInContract = tx.inputs[0].value - 1000;

        // Get Party B Locking Bytecode from NFT commitment.
        bytes partyBLockingBytecode = tx.inputs[0].nftCommitment;

        // Funds are then returned 50-50 to each party.
        require(tx.outputs[0].value == amountAvailableInContract / 2);
        require(tx.outputs[0].lockingBytecode == partyALockingBytecode);
        require(tx.outputs[1].value == amountAvailableInContract / 2);
        require(tx.outputs[1].lockingBytecode == partyBLockingBytecode);
    }
}
  1. The DoS vector here is that the 2nd party can provide arbitrary locking bytecode (in joinContract()) that does not meet lockscript standardness (e.g. 0xbabecafe). Thus, when the contract is attempting to settle (via payout()), it’ll be rejected by the network due to this non-standard lockscript - thereby preventing Party A from ever claiming their funds (the tx is DoS’d).

For such contracts, Lockscript Standardness should probably be enforced in the contract itself. Currently, this can be done by doing something like the following in joinContract() (to support P2PKH or P2SH32):

// Require that the Locking Bytecode is P2SH32 if it is 35 bytes long.
// OP_HASH256[0xaa] OP_DATA_32[0x20] <32 bytes> OP_EQUAL[0x87]
if(lockingBytecode.length == 35) {
  require(lockingBytecode.split(2)[0] == 0xaa20);
  require(lockingBytecode.split(34)[1] == 0x87);
}

// Otherwise, require that the Locking Bytecode is P2PKH.
// OP_DUP[0x76] OP_HASH160[0xa9] OP_DATA_20[0x14] <20 bytes> OP_EQUALVERIFY[0x88] OP_CHECKSIG[0xac]
else {
  require(lockingBytecode.split(3)[0] == 0x76a914);
  require(lockingBytecode.split(23)[1] == 0x88ac);
}

To allow ANY kind of Lockscript that meets standardness would be cumbersome (and OP_CODE expensive though). I don’t have a good solution to this, but just want to put this out there to maybe spur some ideas (and also as a caution to devs).

To spitball a few ideas briefly:

  1. Reconsider lockscript standardness constraints (doing this could introduce even worse side-effects).
  2. Consider an OP_CODE that can check if the lockscript type is supported (I don’t know if this is a good idea unless the lockscript standardness rules make it into consensus).
  3. Short term, consider a CashScript wrapper function to check this (isP2PKHLockingBytecode, isP2SH32LockingBytecode, etc). This doesn’t really solve anything, but might make it a bit easier for Contract Devs (less chance of manual error in the checks).
  4. For now though, it’s probably advisable to do checking similar to the above and having contracts explicitly whitelist the Lockscript types it wants to support.
4 Likes
Non-standard transactions & out of band miner submission
#2

Not that it’s a good idea necessarily, but as a boundary-checking exercise, could you lay out what it would look like, from a narrow perspective of lockscript DOS, if all locking script relay rules were removed? How would that change the DOS vector? Would it eliminate it? Or would it reduce the weight of protecting against it?

I haven’t thought about it much but my gut feel is that at a minimum you would still need to do size checks to ensure that the provided lockscript doesn’t max out your vm limits intentionally to block redemption execution.

1 Like
#3

It would do this because we would likely want this:

We could replace the complicated standardness checks for just a:

require(lockingBytecode.length <= MAX_LOCKSCRIPT_LENGTH)

Another thing to probably be cautious of with that approach would be (as of yet) undefined OP_CODEs making it into the locking script. Maybe that’s not an issue though - a miner could technically already bypass standardness and place whatever lockscript they like there (bypass standardness because standardness is only enforced at the network level?)

Would be nice to get some input on the idea of an OP_ISSTANDARD-like OP_CODE. I’m not a Node Dev and I feel like this OP might be too a bit complex though (could it be abused?).

1 Like
#4

Actually, in my initial example, this might be unnecessary as the Payout Address is getting stashed in the commitment (which allows 40 bytes). So it probably depends upon:

  1. How the LockingBytecode is stored.
  2. Size of the MAX_LOCKSCRIPT_LENGTH.

P2SH32 is 35 bytes - unsure on P2MS (probably bigger)?

1 Like
#5

Saw your post, just wanted to mention two possibly useful ideas here:

Build up with OP_CAT rather than parsing with OP_SPLIT

It’s usually more efficient for contracts to accept user input in the most granular form, then build up state (where necessary) using OP_CAT rather than parsing pre-concatenated inputs with OP_SPLIT. (Note that with the splitting method, you need to carefully check the boundaries of the concatenated components to make sure that e.g. the user can’t fool the contract into accepting some extra bytes appended to a field as part of the next field.)

In this above case, you really only need the user’s script hash (for P2SH) or public key; you already know what the expected output templates should be, so you’re having the user push 2-4 bytes of duplicated data (they’re pushing the standard template opcodes in both their unlocking script and the redeem script) when all you need beyond their hash payload is a boolean: whether or not they’re using P2PKH.

NFTs are usually the most efficient way to pass state

Rather than having the user pass in static public key/script hash information, consider instead just having the contract issue them a “payee” NFT. To get their money out of the system, their wallet just creates a transaction including the NFT. If multiple payouts happen at a particular moment, spin off a payout covenant like Jedex.

With this setup, you often don’t have to validate other outputs at all (saving many bytes), and users have the added benefit of being able to change their security situation without another contract interaction, e.g. they switch wallets, they add a 2FA device, a multisig key-holding employee leaves the company, etc. This also makes composing contracts much more efficient: another contract system can hold the NFT directly rather than needing to create some sort of payout-forwarding P2SH address (to which the contract would identify with a self-issued NFT).

Aside, switching from a “push-based” to a “pull-based” payout strategy resolves issues in a lot domains. Here’s some discussion about why pull-based dividends work better than push:

2 Likes