Skip to content

fix(globe-wallet): use case-insensitive check for native XLM asset - #97

Closed
s6pa1rta3n-lab wants to merge 1 commit into
Orbit-Wal:mainfrom
s6pa1rta3n-lab:fix-issue-86
Closed

fix(globe-wallet): use case-insensitive check for native XLM asset#97
s6pa1rta3n-lab wants to merge 1 commit into
Orbit-Wal:mainfrom
s6pa1rta3n-lab:fix-issue-86

Conversation

@s6pa1rta3n-lab

Copy link
Copy Markdown

Closes #86

Description

In contracts/globe-wallet/src/lib.rs, GlobeWallet::add_asset previously checked is_native using exact case-sensitive equality (asset.code == String::from_str(&env, "XLM")) while duplicate asset detection used case-insensitive equality (codes_match_case_insensitive).

This allowed an attacker or untrusted payload to register a case-variant of XLM with an issuer (e.g. code: "xlm", issuer: Some(fake_issuer)), which bypassed the native asset issuer check (treated as an issued asset) and occupied the case-insensitive namespace slot. Consequently, when the legitimate user later attempted to register real native XLM ("XLM" with no issuer), it was permanently blocked by duplicate detection (AssetAlreadyAdded).

Solution

  1. Canonicalized is_native check in add_asset to use Self::codes_match_case_insensitive(&asset.code, &String::from_str(&env, "XLM")).
  2. Any case-variant of "XLM" with an issuer (Some(...)) is now rejected outright with WalletError::InvalidAssetInfo.
  3. Verified and added unit tests covering:
    • Rejection of lowercase ("xlm") and mixed-case ("Xlm", "xLm") with issuer before native XLM registration.
    • Successful unblocked registration of canonical native XLM ("XLM", None).
    • Rejection of subsequent duplicate case-variants as WalletError::AssetAlreadyAdded.

Design Decision

Decision on case-variant-of-XLM-with-issuer:
Non-native assets registered under any case-variant of "XLM" with an issuer (e.g., "xlm", "Xlm") are rejected outright with WalletError::InvalidAssetInfo. On Stellar/Soroban, XLM is the singular native asset and cannot possess an issuer. Permitting an issued asset under a case variation of "XLM" allows impersonation of native XLM and enables namespace squatting. Aligning is_native with case-insensitive canonicalization ensures all case variations of XLM are subject to the native asset invariant (no issuer allowed).

Definition of Done Checklist

  • 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

Test Output (cargo test --workspace)

running 71 tests
test tests::test_add_asset_overlong_code_fails ... ok
test tests::test_add_and_list_guardians ... ok
test tests::test_add_duplicate_guardian_fails ... ok
test tests::test_add_and_get_assets ... ok
test tests::test_add_asset_empty_code_fails ... ok
test tests::test_add_duplicate_asset_fails ... ok
test tests::test_accept_by_wrong_address_fails ... ok
test tests::test_daily_spent_survives_temporary_ttl_eviction ... ok
test tests::test_initialize ... ok
test tests::test_add_asset_case_variant_duplicate_fails ... ok
test tests::test_cancel_admin_transfer ... ok
test tests::test_cannot_initiate_second_recovery_while_one_pending ... ok
test tests::test_admin_can_cancel_recovery_even_after_quorum ... ok
test tests::test_double_approval_rejected ... ok
test tests::test_execute_recovery_rejects_new_admin_same_as_current_admin ... ok
test tests::test_migrate_user_assets_within_limit_does_nothing ... ok
test tests::test_native_code_with_issuer_is_contradictory_and_rejected ... ok
test tests::test_native_xlm_registration_happy_path ... ok
test tests::test_no_limit_allows_any_spend ... ok
test tests::test_initialize_twice_fails ... ok
test tests::test_migrate_user_assets_requires_admin ... ok
test tests::test_non_native_code_without_issuer_is_underspecified_and_rejected ... ok
test tests::test_non_admin_cannot_add_guardian ... ok
test tests::test_propose_upgrade_accepts_any_hash_without_validation ... ok
test tests::test_pending_admin_cleared_after_accept_admin ... ok
test tests::test_propose_without_accept_keeps_admin_unchanged ... ok
test tests::test_lowercase_and_mixed_case_xlm_with_issuer_rejected_before_native_registered ... ok
test tests::test_max_guardians_limit ... ok
test tests::test_record_spend_boundary_first_second_of_new_day_resets ... ok
test tests::test_record_spend_boundary_drift_awareness ... ok
test tests::test_record_spend_boundary_last_second_of_day_accumulates ... ok
test tests::test_propose_upgrade_requires_admin ... ok
test tests::test_record_spend_bucket_is_integer_division ... ok
test tests::test_record_spend_exact_day_boundary ... ok
test tests::test_max_assets_limit ... ok
test tests::test_migrate_user_assets_trims_excess ... ok
test tests::test_raise_spend_then_lower_limit ... ok
test tests::test_record_spend_within_limit ... ok
test tests::test_record_spend_exceeds_limit_fails ... ok
test tests::test_record_spend_negative_amount_cannot_bypass_daily_limit ... ok
test tests::test_record_spend_negative_amount_fails ... ok
test tests::test_record_spend_zero_amount_fails ... ok
test tests::test_recovery_clears_any_in_flight_normal_admin_transfer ... ok
test tests::test_record_spend_overflow_does_not_poison_later_calls ... ok
test tests::test_remove_asset ... ok
test tests::test_record_spend_rejected_negative_amount_does_not_mutate_state ... ok
test tests::test_remove_guardian_dequorated_proposal_can_requorum_with_fresh_timelock ... ok
test tests::test_remove_guardian_who_never_approved_leaves_proposal_untouched ... ok
test tests::test_remove_nonexistent_asset_fails ... ok
test tests::test_recovery_happy_path_2_of_3 ... ok
test tests::test_remove_guardian_below_threshold_fails ... ok
test tests::test_require_admin_not_initialized ... ok
test tests::test_propose_and_execute_upgrade - should panic ... ok
test tests::test_recovery_rejects_non_guardian ... ok
test tests::test_set_recovery_config_rejects_single_guardian_threshold ... ok
test tests::test_execute_upgrade_with_never_uploaded_hash_traps - should panic ... ok
test tests::test_removed_guardian_approval_no_longer_counts_toward_quorum ... ok
test tests::test_removed_guardian_cannot_initiate_recovery ... ok
test tests::test_spend_limit_set_and_get ... ok
test tests::test_spend_limit_ttl_extension_after_long_idle_period ... ok
test tests::test_revoke_recovery_approval_rejects_non_guardian ... ok
test tests::test_set_recovery_config_rejects_threshold_above_guardian_count ... ok
test tests::test_upgrade_propose_double_fails ... ok
test tests::test_upgrade_rejects_hash_mismatch ... ok
test tests::test_set_recovery_config_requires_min_guardians ... ok
test tests::test_upgrade_requires_admin_and_ready_time ... ok
test tests::test_transfer_admin ... ok
test tests::test_set_recovery_config_rejected_while_recovery_pending ... ok
test tests::test_revoking_approval_below_threshold_resets_timelock ... ok
test tests::test_revoke_recovery_approval_rejects_removed_guardian ... ok
test tests::test_user_assets_ttl_extension_after_long_idle_period ... ok

test result: ok. 71 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 9.34s

     Running tests/record_spend_reentrancy.rs (target/debug/deps/record_spend_reentrancy-9f6bb289bf4090dc)

running 1 test
test two_spends_in_one_host_invocation_accumulate ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.11s

     Running unittests src/lib.rs (target/debug/deps/token_wrapper-11699b1bfd8edd1b)

running 11 tests
test tests::test_allowance_unset_pair_returns_zero ... ok
test tests::test_approve_negative_amount_fails ... ok
test tests::test_approve_past_expiry_fails ... ok
test tests::test_approve_and_allowance ... ok
test tests::test_transfer_from_insufficient_allowance_fails ... ok
test tests::test_transfer_from_expired_allowance_fails ... ok
test tests::test_approve_overwrites_previous_allowance ... ok
test tests::test_transfer_from_happy_path ... ok
test tests::test_transfer_from_zero_amount_fails ... ok
test tests::test_transfer_from_succeeds_exactly_at_expiry_ledger ... ok
test tests::test_transfer_from_rolls_back_allowance_when_underlying_transfer_fails ... ok

test result: ok. 11 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 7.87s

Payout Routing

  • EVM (Base/Arbitrum/Polygon/ETH): 0xF46C9F6d70C50BF81ef3588AB523a90a594a2F89
  • Stellar: GCL6OXAMLD75BMTINA6EMRUDWK5THQUSHMYNLSNBCJAPZJHNYJTUNIBC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant