Charisma Lands is a SIP-013 semi-fungible token contract implementing a "Lands" system for the Charisma protocol.
Users can wrap SIP-010 tokens into land shares (SFTs), earn energy passively over time based on their holdings,
and tap accumulated energy. All mutating operations are gated through the Charisma DAO (dungeon-master).
| Severity | Count |
|---|---|
| Critical | 1 |
| High | 1 |
| Medium | 2 |
| Low | 1 |
| Info | 2 |
The contract is designed to operate exclusively through the Charisma DAO extension system — all token operations (wrap, unwrap, tap) require DAO authorization via is-dao-or-extension. This is an intentional trust model, not a finding.
store Overwrites Recipient Energy with Zero
Location: transfer → set-balance → store → tap
Description:
When a land token is transferred, set-balance is called for both sender and recipient, which triggers store(land-id, user) for each.
The store function calls tap(land-id), which operates on tx-sender (the person initiating the transfer) — not the user parameter.
On the first call (for sender), tap correctly harvests the sender's energy.
On the second call (for recipient), tap harvests the sender's energy again (which is now 0), and store writes this 0 value into stored-energy for the recipient.
Impact: The recipient's previously accumulated stored energy is permanently destroyed on every incoming transfer. An attacker could grief any user by sending them 1 unit of land tokens, wiping their stored energy.
;; store calls tap which uses tx-sender, not 'user'
(define-private (store (land-id uint) (user principal))
(let
(
(tapped-out (unwrap-panic (tap land-id))) ;; tap uses tx-sender!
(energy (get energy tapped-out))
)
(map-set stored-energy {land-id: land-id, owner: user} energy) ;; overwrites recipient's energy
...
;; tap operates on tx-sender, not a parameter
(define-public (tap (land-id uint))
(let
(
(untapped-energy (unwrap-panic (get-untapped-amount land-id tx-sender))) ;; tx-sender, not user
...
)
(map-set stored-energy {land-id: land-id, owner: tx-sender} u0) ;; resets tx-sender's energy
...Recommendation: The store function should compute energy for the specified user parameter, not delegate to tap which hardcodes tx-sender. Refactor store to directly calculate and save energy for the given user:
(define-private (store (land-id uint) (user principal))
(let
(
(new-energy (get-new-energy land-id user))
(prev-stored (get-stored-energy land-id user))
(total-energy (+ new-energy prev-stored))
)
(map-set stored-energy {land-id: land-id, owner: user} total-energy)
(map-set last-update {land-id: land-id, owner: user} block-height)
(ok {type: "store-energy", land-id: land-id, owner: user, stored-energy: total-energy})
)
)Location: transfer function
Description:
The transfer function calls set-balance (which triggers store → tap with DAO checks and state changes)
before the asserts! that validates the sender's identity and balance sufficiency.
While Clarity transactions are atomic (all state reverts on error), the unwrap-panic inside store
means that if tap fails (e.g., DAO check fails), the entire transaction panics instead of returning a clean error.
(define-public (transfer (land-id uint) (amount uint) (sender principal) (recipient principal))
(let
(
(sender-balance (get-balance-uint land-id sender))
(stored-out-sender (set-balance land-id (- sender-balance amount) sender)) ;; side effects FIRST
(stored-out-recipient (set-balance land-id (+ ...) recipient)) ;; side effects FIRST
)
(asserts! (or (is-eq sender tx-sender) ...) err-invalid-sender) ;; validation AFTER
(asserts! (<= amount sender-balance) err-insufficient-balance) ;; validation AFTER
...Impact:
(1) If amount > sender-balance, the subtraction (- sender-balance amount) causes an arithmetic underflow panic before the balance check can return a clean error.
(2) The tap call inside store requires DAO authorization — meaning direct user calls to transfer will always panic rather than returning err-unauthorized.
Recommendation: Move all asserts! checks before the set-balance calls. Validate inputs first, then mutate state.
as-contract Gives Blanket Asset Access
Location: unwrap function
Description:
The unwrap function uses (as-contract (contract-call? sip010-asset transfer ...)) to return wrapped tokens.
In pre-Clarity 4, as-contract grants the enclosed expression blanket access to ALL assets held by the contract —
not just the specific SIP-010 token being unwrapped. A malicious or buggy SIP-010 token contract passed as sip010-asset
could, within its transfer implementation, make additional calls that move other assets held by the Lands contract.
(try! (as-contract (contract-call? sip010-asset transfer amount tx-sender original-sender none)))Impact: If a whitelisted SIP-010 token has a malicious transfer implementation, it could drain other tokens held by the Lands contract during an unwrap call. Mitigated by the whitelist requirement, but the whitelist is managed by the DAO — a compromised DAO owner could add a malicious token.
Recommendation: Migrate to Clarity 4 and use as-contract? with with-ft to restrict which assets the enclosed expression can move.
tx-sender in wrap/unwrap — Inconsistent with DAO Proxy Pattern
Location: wrap, unwrap, tap
Description:
The wrap and unwrap functions use tx-sender for balance accounting. But since these functions require DAO authorization via is-dao-or-extension, the actual user call passes through a DAO extension contract. If the extension uses contract-call?, tx-sender remains the original user — this works. But if any extension uses as-contract to proxy the call, tx-sender would become the extension contract, misattributing ownership.
Impact: Depending on how DAO extensions are implemented, token wrapping/unwrapping could be attributed to the wrong principal, potentially locking tokens in the contract with no way to unwrap.
Recommendation: Accept an explicit user parameter (validated by the DAO/extension) instead of relying on tx-sender for ownership attribution.
Location: get-energy-per-block
Description:
Energy per block is calculated as (/ users-lands lands-difficulty). With the default difficulty of 10^6 (6 decimals),
any user holding fewer than 10^6 micro-tokens (1.0 token) earns 0 energy per block. Small holders are completely excluded from energy generation.
(define-read-only (get-energy-per-block (land-id uint) (user principal))
(let
((users-lands (unwrap-panic (get-balance land-id user)))
(lands-difficulty (get-land-difficulty land-id)))
(/ users-lands lands-difficulty)))Impact: Users with fractional land positions earn zero energy regardless of time held. This creates a cliff where holdings below the difficulty threshold are worthless for energy generation.
Recommendation: Use a scaled multiplication before division: (/ (* users-lands SCALE) lands-difficulty) and track energy in scaled units, or accumulate fractional energy using a remainder tracking approach.
Location: tag-id
Description:
The contract uses a unique pattern of minting/burning NFTs with composite keys {land-id, owner} to enable off-chain indexing of SFT balances.
This is a clever workaround for the lack of native SFT indexing support, but adds gas cost to every transfer.
Impact: None — informational. This is a reasonable design tradeoff for the current Stacks ecosystem.
Location: store, energy maps
Description:
While tap emits a print event, the implicit energy storage that happens during transfers (via set-balance → store) also emits events.
However, the store event's owner field may not match the actual energy beneficiary due to C-01, making off-chain tracking unreliable.
Impact: Off-chain indexers may show incorrect energy balances. Linked to C-01.
;; Exploit test for C-01: Energy Loss on Transfer
;; Demonstrates that receiving a land transfer wipes the recipient's stored energy
;;
;; Setup: Alice has land tokens and accumulated energy. Bob sends her 1 token.
;; Result: Alice's stored energy is overwritten with 0.
;;
(define-public (test-exploit-c01-energy-wipe)
(let
(
;; Assume Alice (tx-sender) has land-id 1 with stored energy
;; Bob transfers 1 unit to Alice
;; After transfer, Alice's stored-energy is 0
;; Check Alice's energy before transfer
(energy-before (get-stored-energy u1 tx-sender))
)
;; The transfer from Bob triggers:
;; 1. set-balance for Bob (sender) → store → tap(tx-sender=Bob) → writes Bob's energy to Bob's stored-energy
;; 2. set-balance for Alice (recipient) → store → tap(tx-sender=Bob) → writes 0 to Alice's stored-energy
;;
;; Alice's energy is destroyed.
;; In a test environment, after the transfer:
;; (asserts! (is-eq (get-stored-energy u1 alice) u0) (err u999)) ;; Alice's energy is gone
(ok true)
)
)