Delegation tokens can be forged
Description
When a delegate is voting on a proposal, their voting power is determined by the amount of delegation tokens they hold for the validator. The tokens have a denom in the format of udelegation_(?P<data>penumbravalid1[a-zA-HJ-NP-Z0-9]+) where the final part represents the validator identity.
The validator they are voting for as well as the voting power is determined by the asset used when voting:
async fn check_unbonded_amount_correct_exchange_for_proposal(
&self,
proposal_id: u64,
value: &Value,
unbonded_amount: &Amount,
) -> Result<()> {
let validator_identity = self.validator_by_delegation_asset(value.asset_id).await?;
// ... snip ...
/// Look up the validator for a given asset ID, if it is a delegation token.
async fn validator_by_delegation_asset(&self, asset_id: asset::Id) -> Result<IdentityKey> {
// Attempt to find the denom for the asset ID of the specified value
let Some(denom) = self.denom_by_asset(&asset_id).await? else {
anyhow::bail!("asset ID {} does not correspond to a known denom", asset_id);
};
// Attempt to find the validator identity for the specified denom, failing if it is not a
// delegation token
let validator_identity = DelegationToken::try_from(denom)?.validator();
Ok(validator_identity)
}
// ... snip ...
impl TryFrom<asset::DenomMetadata> for DelegationToken {
type Error = anyhow::Error;
fn try_from(base_denom: asset::DenomMetadata) -> Result<Self, Self::Error> {
// Note: this regex must be in sync with both asset::REGISTRY
// and VALIDATOR_IDENTITY_BECH32_PREFIX
let validator_identity =
Regex::new("udelegation_(?P<data>penumbravalid1[a-zA-HJ-NP-Z0-9]+)")
.expect("regex is valid")
.captures(&base_denom.to_string())The issue is that there is no caret (^) at the start of the regex when determining extracting the validator's identity, causing the regex to match anywhere in the denom instead of only the start.
Impact
A malicious user could transfer funds into Penumbra from an external chain with a denom such as udelegation_penumbravalid17geftyyhx73w4hns03gwyfrgfdwlde04083udj57qj6s277ndqpskd. This would end up being prefixed in Penumbra with transfer/channel-X/ but would still match the regex.
This fake token could then be used to vote on proposals with any amount of voting power and for any validator.
Recommendations
The regex should be updated to include a caret (^) at the start to ensure that it only matches the start of the denom.
Remediation
This issue has been acknowledged by Penumbra Labs, and a fix was implemented in commit 0d173cf1↗.