crytic / tealer

Static Analyzer for Teal
GNU Affero General Public License v3.0
61 stars 14 forks source link

Update GroupSize detector to consider usage of absolute indexes #144

Closed S3v3ru5 closed 1 year ago

S3v3ru5 commented 1 year ago

MissingGroupSize detector is used to report execution paths that do not check the group size. However, Current recommendation is to use ARC-4 ABI and relative indices to verify the group transaction.

This PR updates the MissingGroupSize detector to consider the usage of absolute indexes. MissingGroupSize now only reports execution paths that use the absolute index to access group-transaction without checking the group size.

Exploit Scenario

def mint_wrapped_algo() -> Expr:
    validations = Assert(
        And(
            Gtxn[0].receiver() == Global.current_application_address(),
            Gtxn[0].type_enum() == TxnType.Payment,
        )
    )
    transfer_op = transfer_wrapped_algo(Txn.sender(), Gtxn[0].amount())
    return Seq([validations, transfer_op, Approve()])

Attacker sends following group transaction:

    0. Payment of 1 million Algos to application address
    1. Call mint_wrapped_algo
    2. Call mint_wrapped_algo 
    ...
    15. Call mint_wrapped_algo

Attacker receives 15 million wrapped-algos instead of 1 million wrapped-algos. Attacker exchanges the\ wrapped-algo to Algo and steals 14 million Algos. More at building-secure-contracts/not-so-smart-contracts/algorand/group_size_check

Recommendation

Use ARC-4 ABI and relative indexes to verify the group transaction.