Asset total supply can be inflated
Description
When local funds are transferred to an external chain, the ICS-20 balance for that denom is updated, but the total token supply does not change.
pub trait Ics20TransferWriteExt: StateWrite {
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,
);When the tokens are returned to Penumbra, the ICS-20 balance is checked, the funds are minted, and the ICS-20 balance is reduced.
// 2. check if we are the source chain for the denom.
if is_source(&msg.packet.port_on_a, &msg.packet.chan_on_a, &denom, false) {
// mint tokens to receiver in the amount of packet_data.amount in the denom of denom (with
// the source removed, since we're the source)
// ... snip ...
// check if we have enough balance to unescrow tokens to receiver
let value_balance: Amount = state
.get(&state_key::ics20_value_balance(
&msg.packet.chan_on_b,
&unprefixed_denom.id(),
))
.await?
.unwrap_or_else(Amount::zero);
if value_balance < receiver_amount {
// error text here is from the ics20 spec
anyhow::bail!("transfer coins failed");
}
state
.mint_note(
value,
&receiver_address,
CommitmentSource::Ics20Transfer {
packet_seq: msg.packet.sequence.0,
// We are chain A
channel_id: msg.packet.chan_on_a.0.clone(),
sender: packet_data.sender.clone(),
},
)
.await
.context("unable to mint note when receiving ics20 transfer packet")?;The issue is that mint_note will call increase_token_supply, which will update the total supply for the asset, even though it was never decreased when the tokens were transferred out, causing the total supply to be higher than it should be.
Impact
A malicious user could exploit this by transferring out delegation tokens for a validator and returning them to increase the total supply, which in turn would increase the voting power of the validator as it is based on the total number of delegation tokens. Repeatedly performing this exploit could allow the validator to gain enough voting power to perform governance actions.
Recommendations
When transferring assets out of Penumbra, decrease_token_supply should be called so that it correctly represents the current number of tokens in the system. Alternatively, increase_token_supply should not be called when the source tokens are returned.
Remediation
This issue has been acknowledged by Penumbra Labs, and a fix was implemented in pull request 4020↗.