search

taproot-wizards / purrfect_vault

Demo of an onchain Bitcoin Vault using OP_CAT
MIT License
48 stars 14 forks source link

enforce input index on trigger and complete transactions #2

Open rot13maxi opened 5 months ago

rot13maxi commented 5 months ago

if you made the first input and first output be a non-vault SPK, you could drain the vault to fees (h/t @dgpv for noticing this issue!). Anywhere we are implicitly checking that the vault is the first input/output, we should actually enforce that. Luckily BIP341 has us commit to the input index, so it's an easy check (https://github.com/taproot-wizards/purrfect_vault/blob/ee7ee57ba9b5ed8fee7382334625bacbe27f8c36/src/vault/signature_building.rs#L268).

dgpv commented 5 months ago

I believe this applies to the cancel transaction, too - use some unrelated utxo for input 0, use covenant utxo for input 1, and the covenant will enforce the sole output = input 0, which is some unrelated utxo. the covenant utxo amount then goes to the fee

wade-liwei commented 4 months ago

Do you have the problem?

image
wade-liwei commented 4 months ago

bitcoin-core.cli -rpcuser=user -rpcpassword=password -regtest getblockcount 0

rot13maxi commented 4 months ago

Hey @wade-liwei can you open a new issue for this?

wade-liwei commented 4 months ago

ok

monlovesmango commented 1 month ago

since this is marked as good first issue, I would like to tackle this if that is ok?

rot13maxi commented 1 month ago

Absolutely! More than happy to review a PR, or if you get stuck and need any pointers or feedback, don’t be shy. Thanks for taking a look!

On Tue, Aug 13, 2024 at 8:47 AM, monlovesmango @.***(mailto:On Tue, Aug 13, 2024 at 8:47 AM, monlovesmango < wrote:

since this is marked as good first issue, I would like to tackle this if that is ok?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

monlovesmango commented 3 weeks ago

ok I just opened a pr for this. sorry it took so long for such a straightforward fix... i had to familiarize myself with the inner workings of taproot in order to craft these drain transactions :)

also, I only updated the trigger and cancel scripts to protect against this exploit as I don't think the complete script is actually vulnerable to this. in the complete script you are serializing one of the prevouts from scratch and then enforcing the order when constructing the entire hashed prevout commitment so its impossible to create a valid transaction with the fee input first. at least I wasn't able to craft a valid complete transaction that drained the wallet (and I was able to create valid trigger and cancel transactions that drained the wallet). does this sound right to you? or is there something I missed?

thanks for letting me work on this! learned so much and was a lot of fun.