Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ Changelog for `odra`.
### Changed
- Refactor of `CEP-95` token implementation.

### Added
- `set_name`, `set_symbol` and `set_decimals` functions to `CEP-18` module.
Comment on lines +9 to +10
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 | Confidence: High

The changelog entry accurately describes the addition but does not warn about the critical behavioral change and security implications. For a public-facing module like CEP-18, changelogs should highlight breaking changes or security requirements. Since these functions introduce mutable metadata (a significant deviation from common token standards) and lack access control, the entry should include a warning or reference to necessary access control setup.

Suggested change
### Added
- `set_name`, `set_symbol` and `set_decimals` functions to `CEP-18` module.
### Added
- `set_name`, `set_symbol` and `set_decimals` functions to `CEP-18` module.
**Warning:** These functions are public by default. To prevent unauthorized changes, ensure the contract uses an access control mechanism (e.g., `Ownable`). Changing `decimals` after deployment can corrupt token accounting.


## [2.4.0] - 2025-09-02
### Changed
- Refactor of `CEP-95` token implementation.
Expand Down
15 changes: 15 additions & 0 deletions modules/src/cep18_token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,21 @@ impl Cep18 {
amount: *amount
});
}

/// Set name of the token.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 | Confidence: High

  • The new public functions directly mutate core token metadata (name, symbol, decimals) without any access control. This is a severe security vulnerability. Any arbitrary external caller can change the token's identity and decimal precision at any time. This can be exploited to: 1. Spoof tokens: An attacker can rename the token to impersonate a legitimate asset, leading to phishing and user confusion. 2. Break financial logic: Changing decimals after deployment can corrupt all existing balances and calculations, as token amounts are stored in base units (e.g., U256). For example, if a token has 100 base units and decimals changes from 2 to 3, the human-readable value changes from 1.00 to 0.100, effectively stealing 90% of each holder's value. This is an irreversible, protocol-breaking change with direct evidence in the snippet.
  • Adding a mutable decimals field fundamentally changes the token's architectural guarantees and violates the principle of least surprise for CEP-18 (a Casper analog of ERC-20). In standard token implementations, decimals is immutable after deployment because: 1. Financial integrity: All on-chain calculations (balances, allowances, transfers) and off-chain integrations (wallets, exchanges) assume a fixed decimal precision. Changing it corrupts the token's arithmetic without migrating balances, a catastrophic failure. 2. Standard compliance: While CEP-18 may not explicitly forbid mutability, prominent standards (ERC-20, BEP-20) treat decimals as immutable, and changing this is a significant, breaking behavioral change for the module. Existing integrations will break if decimals can change. The direct code change introduces this breaking architectural risk with no safeguards (see also the security finding for lack of access control). The impact is high-probability data corruption.
  • The set_decimals function accepts any u8 value (0-255) without validation. While u8 is technically correct, extreme values can cause integration issues or runtime errors in downstream logic (e.g., UI rendering, decimal formatting libraries). Although not a direct runtime crash in the contract, allowing nonsensical values (e.g., decimals = 255) reduces robustness and increases support burden.
  • The function accepts a String without any length validation or sanitization. While not a security issue in the provided snippet (assuming storage handles arbitrary data), excessively long names could bloat storage or cause issues in off-chain tooling that expects reasonable lengths. This is a minor maintainability concern.

Code Suggestion:

// Ensure the contract inherits or uses an access control mechanism (e.g., Ownable).
// Example assumes `self` has an `only_owner` guard or similar.
pub fn set_name(&mut self, name: String) {
    self.ensure_owner(); // Or equivalent access control check
    const MAX_NAME_LENGTH: usize = 50; // Example reasonable limit
    if name.len() > MAX_NAME_LENGTH {
        self.env().revert(Error::NameTooLong);
    }
    self.name.set(name);
}

// Apply same pattern to set_symbol and set_decimals
pub fn set_decimals(&mut self, decimals: u8) {
    self.ensure_owner();
    const MAX_DECIMALS: u8 = 18; // Assuming a reasonable max
    if decimals > MAX_DECIMALS {
        self.env().revert(Error::InvalidDecimals); // Define appropriate error
    }
    self.decimals.set(decimals);
}

pub fn set_name(&mut self, name: String) {
self.name.set(name);
}

/// Set symbol of the token.
pub fn set_symbol(&mut self, symbol: String) {
self.symbol.set(symbol);
}

/// Set decimals of the token.
pub fn set_decimals(&mut self, decimals: u8) {
self.decimals.set(decimals);
}
}

pub(crate) mod utils {
Expand Down