MAKE CSPR.name - Casper Association

The resolver contract allows domain owners to create subdomains without any validation on the subdomain name format. This lack of validation enables the creation of problematic subdomains such as:

• Empty strings (e.g., "..cspr")

• Spaces (e.g., " .cspr")

• Special characters (e.g., "$.cspr")

• No length restrictions

The issue originates from the set_resolution function, which only validates:

• Whether the domain ends with ".cspr"

• Whether the token ID is valid

• Whether the caller is the owner of the token ID

The lack of subdomain validation creates a significant security and usability concern. Malicious actors can create confusing or deceptive subdomains that closely resemble legitimate ones, enabling sophisticated phishing attacks. For instance, an attacker could create subdomains with subtle variations or misleading characters that appear legitimate to users. This not only compromises the security of the naming service but also creates technical challenges for client applications that need to implement proper domain name resolution. Additionally, the absence of validation rules leads to inefficient storage usage as the system must accommodate any arbitrary string as a valid subdomain. This combination of security risks, usability issues, and technical inefficiencies makes the current implementation vulnerable to abuse and potentially harmful to the ecosystem's integrity.

Code Location

Below is the implementation of the set_resolution function:

pub fn set_resolution(&mut self, full_domain: Domain, address: Option<Address>) {
    let env = self.env();
    let token_id = self
        .calculate_token_id(&full_domain)
        .unwrap_or_revert_with(self, ResolverError::InvalidDomain);
    let caller = env.caller();

    if !self.name_token.is_token_valid(token_id) {
        env.revert(ResolverError::ResolutionSetWithInvalidToken);
    }

    if self.owner_of(token_id) != Some(caller) {
        env.revert(ResolverError::ResolutionSetByInvalidOwner);
    }

    let nonce = self.nonce(&token_id);
    self.resolutions
        .set(&(token_id, full_domain.clone(), nonce), address);

    env.emit_event(ResolutionChanged {
        full_domain,
        address,
    });
}

Below is the implementation of the calculate_token_id function:

fn calculate_token_id(&self, full_domain: &str) -> Option<TokenId> {
    let token_name = utils::extract_token_name(&full_domain)?;
    let hash = self.env().hash(token_name);
    Some(U256::from(hash))
}

Below is the implementation of the extract_token_name function within the utils module, which only ensures that the domain ends with CSPR_DOMAIN, which is ".cspr":

pub fn extract_token_name(full_domain: &str) -> Option<String> {
    if full_domain.ends_with(CSPR_DOMAIN) {
        let token_name = full_domain.trim_end_matches(CSPR_DOMAIN);
        token_name.split('.').last().map(|s| s.to_string())
    } else {
        None
    }
}

const CSPR_DOMAIN: &str = ".cspr";

Enforce strict subdomain validation in set_resolution by verifying that the extracted token_name matches a DNS-label regex pattern (e.g., allowing only alphanumeric characters and hyphens, 1–63 characters long, with no empty or special characters). If validation fails, return a custom InvalidSubdomainFormat error.

SOLVED: The MAKE team solved this issue in the specified commit ID.

Users can set any primary name in the reverse resolution without verifying ownership, enabling them to set names they do not control. This can lead to misleading or incorrect reverse records, compromising the trust and integrity of address-to-name mappings.

Code Location

Below is the implementation of the set_primary_name function:

pub fn set_primary_name(&mut self, primary_name: String) {
    // Load currently set primary name.
    let caller = self.env().caller();
    let current_primary_name = self.get_primary_name(&caller);
    // Update primary name.
    self.primary_names.set(&caller, primary_name.clone());
    // Emit event.
    self.env().emit_event(PrimaryNameChanged {
        address: caller,
        old_primary_name: current_primary_name,
        new_primary_name: primary_name,
    });
}

It is recommended to verify that the resolved address of the primary name matches the caller’s address to ensure that only the legitimate owner can set the reverse resolution for their address.

SOLVED: The MAKE team solved this issue in the specified commit ID. The set_primary_name function now ensures that only a valid token owner can set a primary name, and that the name exists in the resolver. Additionally, the get_primary_name function now checks if the token has been invalidated in the resolver and invalidates reverse resolution accordingly.

In computer programming, an overflow occurs when an arithmetic operation attempts to create a numeric value that is outside the range that can be represented with a given number of bits – either larger than the maximum or lower than the minimum representable value. By default, overflow checks are disabled in release mode. Enabling overflow-checks in the workspace ensures that overflow errors are caught even in release builds.

The identified unchecked operations are listed below:

casper-name-contracts/src/contracts/registrar.rs
L176: if block_time > expiration + grace_period {
L231: block_time > token_expiration + grace_period + PENDING_DELETE_PERIOD

The expression block_time > expiration + grace_period is vulnerable to an integer overflow if grace_period is set to an excessively high value. Since overflow checks are disabled by default in release mode, this could cause the sum to wrap around the u64 limit, effectively reversing the comparison logic. As a result, valid prolongation attempts may incorrectly revert with a GracePeriodExpired error.

Similarly, the condition block_time > token_expiration + grace_period + PENDING_DELETE_PERIOD is susceptible to the same overflow issue. This could result in unintended behavior during domain registration or expiration, such as prematurely expiring domains that are still within their valid or grace period—particularly since the expire entry point is publicly accessible.

Although the likelihood of an overflow is low, since it depends on a misconfigured grace_period, enabling overflow checks will prevent such edge-case vulnerabilities.

It is recommended to enable overflow checks for release mode by setting the overflow-checks = true flag in the [profile.release] section of the Cargo configuration to ensure arithmetic operations are checked for overflows even in optimized builds.

Additionally, consider using safe arithmetic methods like checked_add instead of +, and make sure to handle overflows by returning an error or reverting with a clear message.

SOLVED: The MAKE team solved this issue in the specified commit ID. The team implemented a maximum grace period of one year to mitigate the risk of u64 overflow.

The payable functions within the controller and secondary market contracts accept all attached CSPR funds without refunding any excess amount when the attached payment exceeds the amount specified in the voucher. This behavior results in users unintentionally overpaying, as any surplus funds are retained by the contract instead of being reimbursed.

Code Location

Below is the implementation of the collect_cspr_payment function, which is responsible for collecting the attached CSPR tokens in payable functions:

fn collect_cspr_payment<P: Payment>(&self, voucher: &P) {
    let fee_collector = self
        .treasury
        .get_or_revert_with(ControllerError::FeeCollectorNotSet);
    let payment_info = voucher.payment_info();
    if self.env().attached_value() < payment_info.amount {
        self.revert(ControllerError::InsufficientPayment);
    }
    self.env()
        .transfer_tokens(&fee_collector, &payment_info.amount);
    self.env().emit_event(PaymentFulfilled {
        payment_id: payment_info.payment_id.clone(),
        buyer: payment_info.buyer,
        amount: payment_info.amount,
    });
}

It is recommended to refund any excess CSPR to the user to ensure fair handling of funds and prevent unintended overpayments.

SOLVED: The MAKE team solved this issue in the specified commit ID. The transaction logic now reverts if the provided amount does not match the domain price.

The controller and registrar contracts currently lack a pausability mechanism. This feature is crucial for smart contracts, especially those handling significant assets or performing critical operations. A pause functionality allows the contract owner or designated parties to temporarily halt contract operations in case of emergencies, such as discovered vulnerabilities, unexpected behavior, or during upgrades.

It is recommended to implement a pausability mechanism for the aforementioned contracts.

SOLVED: The MAKE team solved this issue in the specified commit ID. The involved contracts now enforce that all mutable, non-admin actions can only be performed when the contract is in an unpaused state.

In the Odra framework, contract schemas are crucial for documenting contract behavior and enabling proper client integration. The framework uses the #[odra::module] attribute macro to define which errors and events should be included in the contract schema. Errors are defined with #[odra::odra_error] and events with #[odra::event], but they must be explicitly included in the module declaration to be part of the schema.

Each contract is missing its corresponding error declaration in the module:

• Controller: Missing ControllerError

• NameToken: Missing NameTokenError

• Resolver: Missing ResolverError

• Registrar: Missing RegistrarError

The incomplete data in the contract schema may result in the following issues:

• The contract schema will be incomplete, missing important error definitions

• Generated documentation will not include all possible error cases

• External clients may not be able to properly handle all error scenarios

It is recommended to include all contract-specific errors in the contract schema to ensure completeness and improve tooling support, developer experience, and documentation accuracy.

For example, in the case of the registrar contract, errors can be explicitly defined in the contract schema as follows:

#[odra::module(errors = [RegistrarError])]
pub struct Registrar

SOLVED: The MAKE team solved this issue in the specified commit ID.

The revoke_whitelist function allows any whitelisted account to revoke the whitelist status of any other address. This is a violation of the principle of least privilege, as whitelisted users should not have administrative capabilities to manage other users' whitelist status. While the contract owner can re-whitelist themselves (since they control the whitelist function), this design flaw could be exploited by malicious whitelisted users to revoke other users' whitelist status, potentially causing disruption to legitimate users and to the availability of critical contract operations.

Code Location

Below is the implementation of the revoke_whitelist function:

pub fn revoke_whitelist(&mut self, address: Address) {
    let caller = self.env().caller();
    self.assert_whitelisted(&caller);
    self.whitelist.set(&address, false);
}

It is recommended to restrict access to the revoke_whitelist function so that only the contract owner can invoke it — mirroring the access control already applied to the whitelist function.

SOLVED: The MAKE team solved this issue in the specified commit ID.

When transferring tokens via admin_transfer or transfer_from by a whitelisted operator where the token's resolver is set to the default resolver, the contract performs two cleanup operations on the same token ID. This occurs because the function first calls self.cleanup(token_id), which performs a cleanup if the token's resolver is the default resolver, and then immediately calls self.default_resolver.cleanup(token_id) again after the transfer. With each cleanup operation, the nonce for the transferred token ID is incremented in the resolver contract. These redundant cleanup operations result in unnecessary gas consumption and cause the nonce to be incremented twice instead of once.

Code Location

Below is a code snippet of the admin_transfer function:

// if called by an operator
if caller != owner {
    self.cleanup(token_id);
    self.token.raw_transfer_from(owner, recipient, token_id);
    // make sure there were no previous records for the new owner
    self.default_resolver.cleanup(token_id);
} else {
    self.token.raw_transfer_from(owner, recipient, token_id);
}

Below is a code snippet of the transfer_from function:

 // if called by an operator
 if caller != owner {
    self.cleanup(token_id);
    self.token.transfer_from(from, to, token_id);
    self.default_resolver.cleanup(token_id);
} else {
    self.token.transfer_from(from, to, token_id);
}

Below is the implementation of the cleanup function:

fn cleanup(&mut self, token_id: U256) {
    let mut metadata = self.wrapped_metadata(token_id);
    let resolver = metadata.resolver().unwrap_or_revert(self);
    if resolver == Some(*self.default_resolver.address()) {
        self.default_resolver.cleanup(token_id);
    } else {
        let default_resolver = *self.default_resolver.address();
        metadata.set_resolver(default_resolver);
        self.set_token_metadata(token_id, metadata.to_vec());
    }
}

Below is the implementation of the resolver::cleanup function:

pub fn cleanup(&mut self, token_id: TokenId) {
    let env = self.env();
    let caller = env.caller();

    if !self.has_role(&DEFAULT_ADMIN_ROLE, &caller) && self.owner_of(token_id) != Some(caller) {
        self.env().revert(ResolverError::UnauthorizedCleanup);
    }
    self.nonces.add(&token_id, 1);

    env.emit_event(ResolutionCleared { token_id });
}

It is recommended to:

• modify the cleanup function to handle the default resolver cleanup in a single operation, and

• remove the redundant self.default_resolver.cleanup(token_id) calls from both admin_transfer and transfer_from functions

The cleanup function should be updated to always perform the default resolver cleanup after handling the metadata updates, ensuring that the cleanup operation is performed exactly once per token transfer.

fn cleanup(&mut self, token_id: U256) {
    let mut metadata = self.wrapped_metadata(token_id);
    let resolver = metadata.resolver().unwrap_or_revert(self);

    if resolver != Some(*self.default_resolver.address()) {
        let default_resolver = *self.default_resolver.address();
        metadata.set_resolver(default_resolver);
        self.set_token_metadata(token_id, metadata.to_vec());
    }
    
    self.default_resolver.cleanup(token_id);
}

SOLVED: The MAKE team solved this issue in the specified commit ID.

The current implementation does not check whether an address is actually whitelisted before attempting to revoke it. While not a security issue, this results in unnecessary gas consumption due to a redundant storage write (self.whitelist.set(&address, false))—writing the default false value for a non-whitelisted address. This could also lead to confusion, as users may expect the function to revert when revoking a non-whitelisted address.

Code Location

Below is the implementation of the revoke_whitelist function:

pub fn revoke_whitelist(&mut self, address: Address) {
    let caller = self.env().caller();
    self.assert_whitelisted(&caller);
    self.whitelist.set(&address, false);
}

It is recommended to ensure that the address is whitelisted before revoking it, otherwise revert with a custom error, such as AddressNotWhitelisted.

SOLVED: The MAKE team solved this issue in the specified commit ID.

The reverse resolver allows users to reassign their primary name even when the new value is identical to the existing one. This results in unnecessary state changes and event emissions, increasing gas costs without any functional benefit.

Code Location

Below is the implementation of the set_primary_name function:

pub fn set_primary_name(&mut self, primary_name: String) {
    // Load currently set primary name.
    let caller = self.env().caller();
    let current_primary_name = self.get_primary_name(&caller);
    // Update primary name.
    self.primary_names.set(&caller, primary_name.clone());
    // Emit event.
    self.env().emit_event(PrimaryNameChanged {
        address: caller,
        old_primary_name: current_primary_name,
        new_primary_name: primary_name,
    });
}

It is recommended to restrict primary name updates to only when the new name differs from the currently set name.

SOLVED: The MAKE team solved this issue in the specified commit ID. The function now exits early with a return statement if the primary name is unchanged, avoiding unnecessary operations.

The registrar contract lacks internal validation to reject structurally invalid domain labels—specifically, labels equal to "cspr" (the top-level domain) or those containing a dot ("."). While the responsibility for providing valid labels lies with the user, and either the controller signer is expected to approve only well-formed names or the admin is expected to register them correctly, there are no safeguards at the smart contract level to enforce this validation.

If either the controller inadvertently signs such a label or the admin mistakenly provides one, the contract will register it, resulting in domains that are technically valid on-chain but unusable within the system’s resolution logic. This includes failure to manage subdomains or resolve the domain as expected. Consequently, users may spend funds on domains that cannot be used functionally, leading to a poor experience and potential loss.

Code Location

Below is the implementation of the register function:

fn register(&mut self, names: Vec<NameMintInfo>) {
    let block_time = self.env().get_block_time();
    for info in names {
        self.assert_token_expires_in_future(info.token_expiration, block_time);

        // Compute token hash.
        let token_id = self.compute_namehash(&info.label);

        // Check if token already exists.
        let token_exists = self.name_token.token_exists(token_id);

        // If token exists and is expired and grace period is over, burn it.
        if token_exists {
            let metadata = self.wrapped_metadata(token_id);
            self.assert_token_expired(metadata.expiration().unwrap_or_revert(self), block_time);
            self.name_token.burn(token_id);
        }

        // Mint token.
        let metadata = NameTokenMetadata::with_resolver(
            &info.label,
            info.token_expiration,
            self.name_token.get_default_resolver(),
        );
        self.name_token
            .mint(info.owner, token_id, metadata.to_vec());
    }
}

It is recommended to enforce strict validation of domain labels before completing the registration process, ensuring that only structurally sound domains can be registered.

SOLVED: The MAKE team solved this issue in the specified commit ID.

Multiple contract initialization and setup functions lack validation to ensure address parameters are valid contract addresses:

• marketplace::init(name_token)

• controller::init(registrar)

• registrar::init(name_token)

• resolver::init(name_token)

• resolver::set_name_token(name_token)

Without proper validation, these functions could be initialized with regular account addresses instead of contract addresses. This could lead to severe issues as the contracts attempt to call functions on non-contract addresses, resulting in failed transactions and potential protocol disruption. The contracts should implement address validation to ensure that only valid contract addresses are accepted during initialization and setup.

It is recommended to verify that the specified addresses are indeed contract addresses by utilizing the is_contract function from the Address type. If an address is not a contract address, the transaction should be reverted. An example implementation is as follows:

if !address.is_contract() {
    self.revert(NameTokenError::InvalidContractAddress);
}

SOLVED: The MAKE team solved this issue in the specified commit ID.

When deploying the registrar contract, the set_admin_role function from the access_control module is invoked to configure the admin role for the controller role (CONTROLLER_ROLE). This admin role determines which entity can grant or revoke the controller role. However, the function is called with the default admin role (DEFAULT_ADMIN_ROLE) as the admin — which is already the default behavior if no admin is explicitly set.

As a result, this invocation has no practical effect and introduces unnecessary code.

Code Location

Below is a code snippet of the init function from the registrar module:

// Setup roles.
self.access_control
    .unchecked_grant_role(&DEFAULT_ADMIN_ROLE, &caller);
self.access_control
    .set_admin_role(&CONTROLLER_ROLE, &DEFAULT_ADMIN_ROLE);

Below is the implementation of the set_admin_role function from the access_control module:

pub fn set_admin_role(&mut self, role: &Role, admin_role: &Role) {
    let previous_admin_role = self.get_role_admin(role);
    self.role_admin.set(role, *admin_role);
    self.env().emit_event(RoleAdminChanged {
        role: *role,
        previous_admin_role,
        new_admin_role: *admin_role
    });
}

Below is the implementation of the get_role_admin function from the access_control module, which is used within grant_role and revoke_role functions:

/// Returns the admin role that controls `role`.
///
/// The admin role may be changed using [set_admin_role()](AccessControl::set_admin_role).
pub fn get_role_admin(&self, role: &Role) -> Role {
    let admin_role = self.role_admin.get(role);
    if let Some(admin) = admin_role {
        admin
    } else {
        DEFAULT_ADMIN_ROLE
    }
}

It is recommended to remove the unnecessary set_admin_role function invocation.

SOLVED: The MAKE team solved this issue in the specified commit ID.

In the registrar module, the compute_namehash function hashes the provided token name and derives a U256 token ID from it. However, the returned value within the resolve function is named token_hash, which is misleading — the variable is not a hash but a U256 representation derived from one. This inaccurate naming can cause confusion for developers and auditors, potentially leading to incorrect assumptions about the type or semantics of the value.

Additionally, in the resolver module, the cleanup function is misleadingly named. Despite implying state-clearing behavior, the function merely increments a nonce and does not remove or modify any stored resolution data. The previous resolutions remain on-chain and are simply rendered inaccessible. This may lead to misunderstanding of the function’s purpose, and could result in overestimating the contract’s ability to reclaim or prune storage.

Code Location

Below is a code snippet of the resolve function from the registrar module:

let token_hash = self.compute_namehash(&token_name);

Below is the implementation of the cleanup function from the resolver module:

pub fn cleanup(&mut self, token_id: TokenId) {
    let env = self.env();
    let caller = env.caller();

    if !self.has_role(&DEFAULT_ADMIN_ROLE, &caller) && self.owner_of(token_id) != Some(caller) {
        self.env().revert(ResolverError::UnauthorizedCleanup);
    }
    self.nonces.add(&token_id, 1);

    env.emit_event(ResolutionCleared { token_id });
}

It is recommended to rename the token_hash variable to token_id to accurately reflect its type and purpose, aligning with the naming convention already used in the register function. Additionally, consider renaming the cleanup function to something more descriptive, such as invalidate_resolutions, to better convey its actual behavior.

SOLVED: The MAKE team solved this issue in the following commit IDs:

• d27f96b

• 859c88f

The registrar, resolver and controller contracts implement admin-only functions that modify critical state variables without emitting events. The affected functions are the following:

• registrar::set_grace_period

• resolver::set_name_token

• controller::set_signer_public_key

• controller::set_treasury

The absence of events prevents external systems or users from tracking critical administrative changes via event logs. This lack of transparency diminishes visibility into admin actions that impact the protocol's behavior.

It is recommended to emit an event whenever important contract state variables are updated.

SOLVED: The MAKE team solved this issue in the specified commit ID.

Several inconsistencies and issues were identified in the project’s inline documentation and comments:

Typographical Errors

The terms "preffered" and "controy" were found in comments within the reverse_resolver and controller modules, and should be corrected to "preferred" and "control", respectively. These typos diminish the perceived quality and professionalism of the codebase.

Misleading or Unclear Comments

In the registrar module, a comment stating "Consider removing this line." was found immediately before granting the controller role to the caller during installation. This suggests uncertainty or a lack of clarity regarding whether the caller should be granted the controller role — especially since the caller already holds the DEFAULT_ADMIN_ROLE, which allows them to grant the controller role themselves later. This could mislead future developers or auditors about the intended behavior.

Also in the registrar module, when reading a token_id from input, the comment reads "Compute token hash". However, no computation occurs at this stage — the value is simply being read.

Additionally, in the name_token module, the following comments incorrectly references the CEP78 standard instead of CEP95:

• "NameToken contract. It is a CEP78 token with additional functionalities."

• "Initializes CEP78 with the given name and symbol."

It is recommended to correct the identified typographical errors and to remove the comment regarding the hash computation.

Additionally, review the logic used to assign the controller role during registrar installation. If role assignment is intentional, consider removing the ambiguous comment ("Consider removing this line."); if the assignment is redundant, remove the assignment itself.

Finally, replace all references to CEP78 with CEP95.

SOLVED: The MAKE team solved this issue in the specified commit ID.

It was identified that the resolver address is being unnecessarily cloned each time it is accessed via its dedicated getter function. Since the resolver address is of type Option<Address> and implementing the Copy trait, cloning it introduces unnecessary overhead without providing any functional benefit. This results in minor inefficiencies and may indicate a misunderstanding of the type's intended semantics.

Code Location

Below is the implementation of the resolver function found in the data_structures file:

pub fn resolver(&self) -> OdraResult<Option<Address>> {
    Ok(self.resolver.clone())
}

It is recommended to remove the unnecessary clone.

SOLVED: The MAKE team solved this issue in the specified commit ID.