Description

When a denom is transferred to an external chain via IBC, the ICS-20 balance for it is updated to keep track of the total amount that has been transferred out:

async fn withdrawal_execute(&mut self, withdrawal: &Ics20Withdrawal) -> Result<()> {
    // create packet, assume it's already checked since the component caller contract calls `check` before `execute`
    let checked_packet = IBCPacket::<Unchecked>::from(withdrawal.clone()).assume_checked();

    let prefix = format!("transfer/{}/", &withdrawal.source_channel);
    if !withdrawal.denom.starts_with(&prefix) {
        // we are the source. add the value balance to the escrow channel.
        let existing_value_balance: Amount = self
            .get(&state_key::ics20_value_balance(
                &withdrawal.source_channel,
                &withdrawal.denom.id(),
            ))
            .await
            .expect("able to retrieve value balance in ics20 withdrawal! (execute)")
            .unwrap_or_else(Amount::zero);

        let new_value_balance = existing_value_balance + withdrawal.amount;
        self.put(
            state_key::ics20_value_balance(&withdrawal.source_channel, &withdrawal.denom.id()),
            new_value_balance,
        );
    }

This balance is then checked when a denom is transferred back to ensure that it is not possible to transfer in more than was transferred out.

The issue is that the withdrawal amount is added to the existing balance without using checked_add, so it is possible for the new balance to overflow the u128 back to zero.

Impact

If the overflow occurs, it would prevent the majority of the denom from being transferred back to Penumbra as the ICS-20 balance would be too small to allow it.

Generally, it should not be possible to reach this condition as the total supply for a denom should always be under u128; however, it could be combined with another bug, allowing it to be exploited.

Recommendations

The new balance should be calculated using checked_add to ensure that it does not overflow and to bring it in line with the rest of the balance calculations in the codebase.

Remediation

This issue has been acknowledged by Penumbra Labs, and a fix was implemented in commit 53d1280e↗.