Transaction Input Bug

[colin-axner]

Since inputCount is used to determine how many inputs were used in a transaction, if a user were to create a transaction with Input 1 zeroed out and Input 2 with a valid reference to a utxo. They would not be able to successfully withdraw from that transaction as Validate.sol would attempt to check for two valid inputs.

Perhaps enforcing valid transaction structuring on the child chain would be the best way to avoid this. Any input 1 field with all zeros should also imply that input 2 fields are also zero.

-Blockchain @ Berkeley Researcher

[smartcontracts]

Yep, confirming this issue.

The problem is here:

        if (inputCount == 0) {
            return msg.sender == ECRecovery.recover(confirmationHash, confSig1);
        }
        if (inputCount < 1000000000) {
            return ECRecovery.recover(txHash, sig1) == ECRecovery.recover(confirmationHash, confSig1);
        } else {
            bytes memory confSig2 = ByteUtils.slice(sigs, 195, 65);
            bool check1 = ECRecovery.recover(txHash, sig1) == ECRecovery.recover(confirmationHash, confSig1);
            bool check2 = ECRecovery.recover(txHash, sig2) == ECRecovery.recover(confirmationHash, confSig2);
            return check1 && check2;
        }

inputCount is determined by multiplying the block number of the second input by 1000000000 and then adding the block number of the second input. There are four potential cases here, both are 0, both are non-0, or only one is non-0. In this last case we need to check one of two signatures. We can't always be sure that the first input of the two is going to be the non-0 one.

We could fix by checking if the first 9 digits of inputCount is > 0, something like:

if ((inputCount % 10 ** 9) > 0) {
    // check first sig
}
if (inputCount > 1000000000) {
    // check second sig
}