Incorrect ICS-20 balance on time-out
Description
When an ICS-20 transfer is attempted but not received by the destination chain in time, a time-out packet is sent back to the source chain so that the funds can be returned:
async fn timeout_packet_inner<S: StateWrite>(mut state: S, msg: &MsgTimeout) -> Result<()> {
// ... snip ...
if is_source(&msg.packet.port_on_a, &msg.packet.chan_on_a, &denom, true) {
// sender was source chain, unescrow tokens back to sender
// ... snip ...
state
.mint_note(
value,
&receiver,
CommitmentSource::Ics20Transfer {
packet_seq: msg.packet.sequence.0,
channel_id: msg.packet.chan_on_a.0.clone(),
sender: packet_data.sender.clone(),
},
)
.await
.context("couldn't mint note in timeout_packet_inner")?;
// ... snip ...
let new_value_balance =
value_balance
.checked_sub(&withdrawal.amount)
.ok_or_else(|| {
anyhow::anyhow!("underflow subtracting value balance in ics20 withdrawal")
})?;
state.put(
state_key::ics20_value_balance(&msg.packet.chan_on_a, &denom.id()),
new_value_balance,
);
} else {
state
.mint_note(
value,
&receiver,
// NOTE: should this be Ics20TransferTimeout?
CommitmentSource::Ics20Transfer {
packet_seq: msg.packet.sequence.0,
channel_id: msg.packet.chan_on_a.0.clone(),
sender: packet_data.sender.clone(),
},
)
.await
.context("failed to mint return voucher in ics20 transfer timeout")?;
}
Ok(())
}In the first branch, the funds are minted back to the sender and the ICS-20 balance for the denom is reduced. In the else block, where the denom was being returned to the source chain, the funds are minted back to the sender but the ICS-20 balance is not updated, causing it to be lower than it should be.
Impact
A malicious user could exploit this by continually transferring out a small amount of a popular denom from an external chain with a low time-out height, causing the ICS-20 balance to be reduced to zero. This would prevent anyone holding that denom in Penumbra from being able to transfer it back to the original chain.
Recommendations
The ICS-20 balance for the denom should be increased by withdrawal.amount after the funds are minted back to the sender.
Remediation
This issue has been acknowledged by Penumbra Labs, and a fix was implemented in commit b1b1051c↗.