Description

When an incoming IBC transfer is received, the denom is checked to see if Penumbra was the original source chain, and if so, the prefix is removed to get the original denom:

// 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)
    let prefix = format!(
        "{source_port}/{source_chan}/",
        source_port = msg.packet.port_on_a,
        source_chan = msg.packet.chan_on_a
    );

    let unprefixed_denom: asset::DenomMetadata = packet_data
        .denom
!        .replace(&prefix, "")
        .as_str()
        .try_into()
        .context("couldnt decode denom in ICS20 transfer")?;

    let value: Value = Value {
        amount: receiver_amount,
        asset_id: unprefixed_denom.id(),
    };

The issue is that the replace function will remove all occurrences of the prefix, not only the first one.

Impact

In the normal flow, a transfer between multiple chains would look like the following:

1. Chain A transfers CoinA to Penumbra; it ends up with the denom transfer/channel-0/CoinA.

2. Penumbra transfers transfer/channel-0/CoinA to Chain B; it ends up with the denom transfer/channel-1/transfer/channel-0/CoinA.

3. Chain B transfers transfer/channel-1/transfer/channel-0/CoinA back to Penumbra, the prefix transfer/channel-1 is stripped, and the denom is now transfer/channel-0/CoinA.

If Chain A then transfers Cointransfer/channel-1/A to Penumbra and then to Chain B, the denom would be transfer/channel-1/transfer/channel-0/Cointransfer/channel-1/A. When this is transferred back, both instances are replaced and the denom becomes transfer/channel-0/CoinA.

Chains such as Osmosis allow users to create custom denoms in the form of factory/{creator address}/{subdenom}, so a malicious user could create two denoms that could be reduced to the same denom on /Penumbra and withdrawn.

Recommendations

Only the first occurrence of the prefix should be removed.

Remediation

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