🛡️ Unrestricted Access to Break_Stone if Deployed without admin

[ccamel]

Note

Severity: Medium
target: v5.0.0 - Commit: cde785fbd2dad71608d53f8524e0ef8c8f8178af
Ref: OKP4 CosmWasm Audit Report v1.0 - 02-05-2024 - BlockApex

Description

The Law Stone contract utilizes the store_object function of the Objectarium to store .pl files containing rule sets. A key function within this contract is break_stone, designed to modify or remove these rules by unpinning or forgetting them. The function implements an admin check to restrict access: if an admin address is configured, it ensures that only the admin can execute the function by comparing the info.sender with the admin address.

Specifically, the function logic:

{
  Some(admin_addr) if admin_addr != info.sender => Err(ContractError::Unauthorized),
  _ => Ok(()),
};

indicates that if an admin is set and the caller is not the admin, the function will deny access. However, if no admin is defined, particularly in deployments initiated with the --no-admin flag in CosmWasm CLI, the function proceeds without any access restrictions. This lack of checks in the absence of an admin means that anyone can invoke break_stone and potentially disrupt the rule enforcement by the Law Stone, impacting the governance or operational constraints enforced by these rules.

Recommendation

• Enforce Admin Configuration: Modify the contract deployment process to require an admin address explicitly. This change would prevent the contract from being deployed without admin oversight.
• Default Admin Fallback: Implement a default admin setting that can be used if no specific admin is provided during deployment, ensuring there's always some level of controlled access.

[amimart]

It's true that only that instantiated without admin the break_stone message can be called by anyone, even if it's the caller's responsibility I'm not opposed to fallback to the address at the origin of the instantiate or to prevent law stone creation, let me know you point of view @bdeneux @ccamel

[ccamel]

Yes, I agree. I think the best approach is the one that offers the best user experience while maximizing security. In this case, I’d tend to use the creator address (the address that instantiated the contract) instead of the admin. It makes sense to me that the creator remains sovereign of their stone.