Skip to content

[Bug]: is_native uses case-sensitive equality while duplicate-detection is case-insensitive — a squatted 'xlm' entry can permanently block real native XLM registration #86

Description

@ndii-dev

Context

GlobeWallet::add_asset, contracts/globe-wallet/src/lib.rs. Issue #29 made asset-code duplicate detection case-insensitive (codes_match_case_insensitive) so "USDC" and "usdc" can't both be registered. The native-XLM check was never updated to match.

Problem

pub fn add_asset(env: Env, user: Address, asset: AssetInfo) -> Result<(), WalletError> {
    ...
    let is_native = asset.code == String::from_str(&env, "XLM"); // exact, case-SENSITIVE
    if is_native {
        if asset.issuer.is_some() { return Err(WalletError::InvalidAssetInfo); }
    } else {
        if asset.issuer.is_none() { return Err(WalletError::InvalidAssetInfo); } // requires an issuer
    }
    ...
    for i in 0..assets.len() {
        if Self::codes_match_case_insensitive(&assets.get(i).unwrap().code, &asset.code) { // case-INSENSITIVE
            return Err(WalletError::AssetAlreadyAdded);
        }
    }
    ...
}

Two different equality functions, two different canonicalizations, in the same function, checking related things.

Walk through registering "xlm" (lowercase) with a real-looking issuer before the user has ever registered real native "XLM":

  1. is_native — exact match against "XLM" fails (it's "xlm"), so this is not treated as native.
  2. Falls to the else branch: asset.issuer.is_none() is false (an issuer was supplied) — passes. A non-native, issuer-bearing asset with code "xlm" is now registered.
  3. The user (or their client, populating a field from a deep link / QR code / SEP-7 payload — the mobile app added exactly this kind of untrusted-input asset-add flow recently) later tries to register the real native "XLM" (no issuer).
  4. The duplicate-detection loop runs codes_match_case_insensitive("xlm", "XLM")true (case-insensitive) → rejected as AssetAlreadyAdded.

The fake, issuer-bearing "xlm" entry permanently occupies the case-insensitive namespace slot the real native asset needs, and the real native XLM registration is blocked for good (nothing removes the fake entry automatically, and the user has no reason to suspect their own prior "xlm" entry is why "XLM" won't register).

Reproduction steps

#[test]
fn test_lowercase_xlm_with_issuer_blocks_real_native_xlm() {
    let (env, _cid, admin, client) = setup();
    let user = Address::generate(&env);
    let fake_issuer = Address::generate(&env);

    // Registers successfully -- is_native's exact match against "XLM" fails
    // for "xlm", so it's treated as a normal issued asset.
    let fake_xlm = AssetInfo { code: String::from_str(&env, "xlm"), issuer: Some(fake_issuer) };
    client.add_asset(&user, &fake_xlm);

    // The user now tries to add the real, native XLM.
    let real_xlm = AssetInfo { code: String::from_str(&env, "XLM"), issuer: None };
    let result = client.try_add_asset(&user, &real_xlm);
    assert_eq!(result, Err(Ok(WalletError::AssetAlreadyAdded))); // blocked, permanently, by the fake entry
}

Impact

A malicious "add this asset" link (plausible via the mobile app's SEP-7/QR-driven add-asset flows) can pre-squat a case-variant of "XLM" with an attacker-controlled issuer before the user ever registers native XLM, permanently denying that user the ability to register/track their actual native balance through this contract's registry — a targeted griefing vector, and a confusing one to diagnose since nothing about the error (AssetAlreadyAdded) hints that the blocker is a case-variant, not literally "XLM" itself.

Suggested fix

Use the same case-insensitive comparison for the native-asset check that duplicate-detection already uses: Self::codes_match_case_insensitive(&asset.code, &String::from_str(&env, "XLM")). Once that's consistent, decide (and document) whether non-native assets with a case-variant of "XLM" should be rejected outright regardless of issuer, since they're indistinguishable from an attempt to impersonate the native asset.

Definition of done

  • is_native check uses the same canonicalization as the duplicate-detection loop
  • Test proving a lowercase/mixed-case "xlm" (with any issuer) is rejected the same way an uppercase duplicate would be, before the real native XLM is ever registered
  • Test proving native XLM registration still succeeds normally when no case-variant squat exists (no regression to the happy path)
  • Decision on whether case-variant-of-XLM-with-issuer should be rejected outright, written out in the PR
  • cargo test --workspace output pasted

Metadata

Metadata

Assignees

No one assigned

    Labels

    GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardThird CampaignCampaign: Third CampaignbugSomething isn't workingvery hardDifficulty: very hard

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions