Auditing Clarity: Our Journey With Clara AI

mike cohenmike cohen
22 min read

You’ve invested months of time building your smart contracts, honing the features, building UI and API layers. You’ve written hundreds of unit tests and integration tests on devnet and have pushed everything to testnet for early adopters to happy test and find edge cases. You’re ready for the big push to mainnet but because your contracts can custodian large amounts of user assets, NFTs FTs etc, you’d like the security of a full smart contract audit.

But here’s the snag - a full audit is going to cost 20 - 40k USD! You don’t have the funds. You need to get to mainnet to demonstrate traction and revenue to investors where you might be able to attract the funding needed to cover this cost. So you are now caught between a rock and a hard place - you can’t afford to lose the significant investment you and the team have put in but you also fully understand the risks and want the safest possible launch for your project.

Fortunately, we discovered Clara AI — an emerging AI-powered auditing platform for Clarity contracts. Clara AI offered to run a full audit of our system free of charge, enabling us to move toward mainnet with greater confidence. This blog documents the experience and our responses to their findings.

Summary of Findings

BigMarket DAO’s Clarity contracts were reviewed via an in-depth, AI-driven audit by the Clara AI platform, which identified critical areas for improvement.

All findings in the report are addressed point by point. We start with the recommendations where action has been taken resulting in a change to the contract code. The commits where code has been updated to satisfy a particular finding can be found in the BigMarket DAO GitHub commit stream. We aim for our findings and responses to provide value for other projects evaluating the technology and for and also as a Clarity smart contract programming language learning resource.

Overall, Clara AI found a number of legitimate issues in our contracts which have now been addressed. Additionally, we found that the process itself led us to engage with our contract layer with a fresh perspective and this has helped tighten the code further.

The report has given us a much higher degree of confidence in our Clarity contracts than we would otherwise have achieved.

Findings

The findings are split between action taken and no action required. We hope the discussion of each item contributes to the open and transparent discussion of smart contract code within the Stacks developer community.

Each finding is copied from the report with sections;

  • How it can fail

  • What we Recommend

The Response section is the BigMarket response for the issue and indicates the action undertaken to fix the issue.

Findings: Action Taken

Findings where we have taken some action.

[24] Access Control

  • critical

Ensure Least Privilege Principle in Smart Contract Access Control

Implement the principle of least privilege in Clarity smart contracts. Use define-public for functions that require external access, and define-private for internal operations. Validate the tx-sender or contract-caller principal in public functions to ensure proper authorization. Utilize response types (ok and err) to control execution flow and prevent unauthorized access. Carefully manage permissions in contract-call? interactions. Regularly audit and test access control mechanisms to identify and rectify potential vulnerabilities.

How it can fail

The create-market-vote function lacks proper access control and authorization
mechanisms. While comments suggest that users need stake to propose votes, there are
no explicit checks to verify the caller's permissions, stake requirements, or
authority. The function can be called by any user without proper verification of
their eligibility.

(define-public (create-market-vote...

What we Recommend

Implement explicit access control checks at the beginning of the function using
assertions to verify that the tx-sender has sufficient stake or appropriate role
permissions. Consider implementing role-based access control patterns or a whitelist
of authorized users. Add (asserts! (is-authorized-caller tx-sender) error-code)
before performing sensitive operations.
Response

Response

The access control is determined in the market contract - not the voting contract. The comments in the voting contract are out of date and have been updated to reflect this. For example the market contract all contain the line;

;; market contract..
(stake-data (unwrap! (map-get? stake-balances { market-id: market-id, user: disputer }) err-disputer-must-have-stake))

which ensures the user is staking in the given market.

What is missing is a check that the market contract itself is part of the DAO.

The following has been added to create-market-vote to ensure the market contract is an enabled DAO extension.

Action: add enabled extension check on ‘market’ contract

;; Verify that the market is a registered extension
(asserts! (contract-call? .bigmarket-dao is-extension (contract-of market)) err-invalid-extension)

[25] Access Control

  • high

Identification of External Libraries Critical for Smart Contract Operation

How it can fail The function accepts any contract implementing the prediction-market-trait without validating its authenticity or trustworthiness. It blindly calls the external contract's dispute-resolution function without verifying the contract's behavior or state, potentially allowing interaction with malicious contracts.

What we Recommend Implement a whitelist of approved market contract addresses. Add explicit checks to verify the market contract's state before calling its functions. Consider using a registry pattern where trusted contracts are registered by authorized principals. Add assertions to validate the behavior and responses from external contract calls

 ;; File: contracts/extensions/bme021-0-market-voting.clar
(define-public (create-market-vote

Response

Good issue. This has been addressed by adding a check on the market contract;

;; Verify that the market is a registered extension
(asserts! (contract-call? .bigmarket-dao is-extension (contract-of market)) err-invalid-extension)

This is preferable to an allow list type guard as the extension will have undergone community scrutiny to be registered as a valid DAO extension (very similar to a registry but includes DAO oversight).

[8] Business Logic

  • medium

Ensure contract enforces appropriate business limits

What we Recommend Implement explicit validation and bounds checking for both parameters: start-burn-height: Add assertions to ensure it is greater than or equal to the current block height duration: Add assertions to ensure it falls within acceptable minimum and maximum

Response

Action: assertion on start block to be implemented


[13] Business Logic

  • medium

Ensure contract enforces appropriate business limits

What we Recommend Add reasonable boundary checks by implementing minimum and maximum values for signals-required parameter (ensure signals > 0 and less than a maximum value). Consider implementing limits on executive team size. Use var-get to validate current values before allowing changes that could violate business logic constraints

Response

Action: update checks on signals to enforce bounds


[23] Business Logic

  • medium

Ensure contract enforces appropriate business limits

File Name: contracts/extensions/bme010-0-token-sale.clar

How it can fail The buy-ido-tokens function is missing individual purchase limits. While total supply limits are verified, users can purchase unlimited tokens in single or multiple transactions, which could lead to IDO domination by wealthy participants and prevent fair token distribution

What we Recommend Implement per-user purchase limits by adding a maximum purchase amount per user. Store this in the contract state and validate against it in each purchase. Add assertions to check that a user's total purchases (current + new) don't exceed this limit. Consider implementing time-based purchase windows or tiered pricing to further promote fair distribution.

(define-public (buy-ido-tokens (stx-amount uint))
  (let (
    (stage (var-get current-stage))
    (stage-info (unwrap! (map-get? ido-stage-details stage) err-invalid-stage))
    (bmg-price (get price stage-info))
    (max-supply (get max-supply stage-info))
    (tokens-sold (get tokens-sold stage-info))
    (sender tx-sender)
        (cancelled (get cancelled stage-info))
    (current-stake (default-to u0 (map-get? ido-purchases {stage: stage, buyer: tx-sender})))
    (tokens-to-buy (* stx-amount bmg-price))
    )

    ;; Ensure enough supply remains
    (asserts! (<= (+ tokens-sold tokens-to-buy) max-supply) err-stage-sold-out)
    (asserts! (not cancelled) err-stage-cancelled)

    ;; Accept STX payment
    (try! (stx-transfer? stx-amount tx-sender .bme006-0-treasury))

    ;; Mint tokens directly to the buyer
    (try! (as-contract (contract-call? .bme000-0-governance-token bmg-mint tokens-to-buy sender)))

    ;; Update stage details
    (map-set ido-stage-details stage (merge stage-info {tokens-sold: (+ tokens-sold tokens-to-buy)}))
    (map-set ido-purchases {stage: stage, buyer: tx-sender} (+ current-stake tokens-to-buy))

    (print {event: "ido-purchase", buyer: tx-sender, stage: stage, tokens: tokens-to-buy, stx-amount: stx-amount})

    (ok tokens-to-buy)
  )
)
(define-public (buy-ido-tokens (stx-amount uint))
  (let (
    (stage (var-get current-stage))
    (stage-info (unwrap! (map-get? ido-stage-details stage) err-invalid-stage))
    (bmg-price (get price stage-info))
    (max-supply (get max-supply stage-info))
    (tokens-sold (get tokens-sold stage-info))
    (sender tx-sender)
        (cancelled (get cancelled stage-info))
    (current-stake (default-to u0 (map-get? ido-purchases {stage: stage, buyer: tx-sender})))
    (tokens-to-buy (* stx-amount bmg-price))
    )

    ;; Ensure enough supply remains
    (asserts! (<= (+ tokens-sold tokens-to-buy) max-supply) err-stage-sold-out)
    (asserts! (not cancelled) err-stage-cancelled)

    ;; Accept STX payment
    (try! (stx-transfer? stx-amount tx-sender .bme006-0-treasury))

    ;; Mint tokens directly to the buyer
    (try! (as-contract (contract-call? .bme000-0-governance-token bmg-mint tokens-to-buy sender)))

    ;; Update stage details
    (map-set ido-stage-details stage (merge stage-info {tokens-sold: (+ tokens-sold tokens-to-buy)}))
    (map-set ido-purchases {stage: stage, buyer: tx-sender} (+ current-stake tokens-to-buy))

    (print {event: "ido-purchase", buyer: tx-sender, stage: stage, tokens: tokens-to-buy, stx-amount: stx-amount})

    (ok tokens-to-buy)
  )
)

Response

This is a valid and interesting observation. We have begun to move away from a token sale - favouring a reputation based reward system that automatically caps the tokens available. However this is not set in stone at this point and we will discuss this further.

Despite the possibility of account splitting, implementing an upper limit per account still sends a signal to the community and has value.

Potential solution;

(define-constant max-user-ido-purchase u500000000000) ;; 500,000 tokens (with 6 decimals)
;; or maybe a data var with dao setter
(let (
  ;; ... other vars ...
  (current-stake (default-to u0 (map-get? ido-purchases {stage: stage, buyer: tx-sender})))
  (tokens-to-buy (* stx-amount bmg-price))
)
  (asserts! (<= (+ current-stake tokens-to-buy) max-user-ido-purchase) err-user-limit-reached)
  ;; ... rest of function ...
)

Action: under advisement


[26] Business Logic

  • medium

Ensure contract enforces appropriate business limits

File Name: contracts/extensions/bme021-0-market-voting.clar

How it can fail The function lacks proper parameter validation and limits. There are no bounds on num-categories or the number of polls a user can create, which could lead to resource abuse. The empty-votes values aren't validated to ensure they're actually empty (zeros). Error constant names are mismatched with their validation checks.

What we Recommend Implement maximum limits and range validation for num-categories using asserts! to prevent resource abuse. Validate that empty-votes contains zeros if that's the expected initial state. Add rate limiting for poll creation per user. Rename error constants to match their validation logic or adjust the validation to match error names. Ensure voting-duration is within reasonable bounds.

Response

Pass num-categories to the market contract and assert it has the correct number of categories.

Action: addressed


[30] Business Logic

  • medium

Ensure contract enforces appropriate business limits

File Name: contracts/extensions/bme023-0-market-bitcoin.clar Function Name: process-stake-transfer

How it can fail The function has multiple validation issues: it lacks maximum amount limits for transfers, doesn't validate fee-bips range, and doesn't verify if the transfer amount remains meaningful after fee deduction. This could lead to arbitrarily large transfers and excessive fees.

What we Recommend Implement an upper bound check for the amount parameter using assert! to prevent unreasonably large transfers. Add validation for fee-bips to ensure it stays within acceptable limits. Include a check to verify that transfer-amount remains above a minimum threshold after fee deduction. Consider using constants to define these business limits for consistency.

Response

Valid comments. Fixed by adding assertions;

      (asserts! (<= amount u1000000000000) err-amount-too-high)
      (asserts! (<= (var-get dev-fee-bips) u1000) err-fee-too-high) ;; max 10%
      (asserts! (> transfer-amount u0) err-amount-too-low)

Action: addressed


Findings: No Action Taken

[9] Access Control

  • critical

Improper use of tx-sender for access control in Clarity smart contracts

Location: bme003-0-core-proposals.clar Issue: Uses tx-sender instead of contract-caller, enabling confused deputy attacks.

Code:

        ;; File: contracts/extensions/bme003-0-core-proposals.clar
(define-public (is-dao-or-extension)
        (ok (asserts! (or (is-eq tx-sender .bigmarket-dao) (contract-call? .bigmarket-
dao is-extension contract-caller)) err-unauthorised))
)

Response

There is more to this than meets the eye.

(is-eq tx-sender .bigmarket-dao)

Does confirms both that .bigmarket-dao is involved — and how it's involved - ie that it took control of the sending context:

.bigmarket-dao explicitly called this contract using as-contract, taking ownership of the transaction context.

Whereas:

(is-eq contract-caller .bigmarket-dao)

confirms that .bigmarket-dao is the immediate caller — but not whether it took responsibility via as-contract

Action: no action


[28] Access Control

  • critical

Ensure Least Privilege Principle in Smart Contract Access Control

How it can fail: The force-resolve-market function has a critical security vulnerability due to lack of access control mechanisms. The function is publicly accessible and allows any user or contract to force-resolve a disputed market after the timeout period without any authorization validation. This enables unauthorized parties to manipulate market outcomes and resolution states, as the function performs critical state changes (updating resolution state and marking markets as concluded) without verifying the caller's identity or permissions.

What we Recommend: Implement proper access control by adding authorization checks at the beginning of the function using tx-sender or contract-caller validation. Create a permission system (like a map or data variable) to track authorized addresses (admins, arbitrators, market creators) and verify the caller's identity before allowing the market resolution action. Add assertions like (asserts! (is-authorized tx-sender) err-unauthorized) where is-authorized could be a function that checks against an admin list or other role-based authorization mechanism. Consider restricting the function to specific roles like administrators, market creators, or governance mechanisms.

        ;; File: contracts/extensions/bme023-0-market-bitcoin.clar
(define-public (force-resolve-market (market-id uint)) ...

Response

The method implements a permissionless failsafe pattern and has no access control intentionally. It enforces the closure of a market in the case where a market vote is not concluded after a suitable block time expiration.

See conditions;

asserts! (> elapsed (var-get resolution-timeout)): only after the timeout
asserts! (is-eq (get resolution-state md) RESOLUTION_DISPUTED): only if it's in a disputed state

Action: no action


[34] Access Control

  • critical

Ensure Least Privilege Principle in Smart Contract Access Control

How it can fail The resolve-market-undisputed function has a critical security vulnerability due to missing access control mechanisms. The function allows any address to resolve a market and change its state to concluded/resolved once the dispute window has elapsed and the market is in RESOLUTION_RESOLVING state. There are no authorization checks to verify if the caller has the proper permissions to perform this sensitive operation, which could lead to unauthorized market resolutions and potential manipulation of financial outcomes

What we Recommend Implement robust access control mechanisms at the beginning of the function by:

1. Adding authorization checks that verify tx-sender or contract-caller against authorized principals

2. Using assertions to validate caller's permissions (e.g., (asserts! (is-authorized contract-caller) ERR_UNAUTHORIZED))

3. Maintaining an admin list or permission map to track authorized principals 4. Consider implementing either

- Role-based access control system - Multi-signature approach for sensitive operations - Trait-based approach for flexible permission management

5. Verify the caller is an appropriate entity (admin, market creator, or designated resolver

;; File: contracts/extensions/bme023-0-market-predicting.clar
(define-public (resolve-market-undisputed (market-id uint))

Response

The contract intentionally (permissionless failsafe) allows anyone to resolve the market here providing

  1. Market must be in RESOLUTION_RESOLVING: no dispute submitted yet

  2. Dispute window must have expired: no one raised an objection in time

If those are true, the contract permits anyone to call this and finalize. Please note this method DOES NOT CHANGE the result to the market ad there is no security risk leaving its access open.

Action: no action


[42] Business Logic

Ensure contract enforces appropriate business limits

How it can fail The transfer function lacks maximum amount limits, allowing unlimited token transfers in single transactions. Despite Clarity's built-in overflow protection and balance checks, the absence of explicit business limits poses risks to token economics and user protection

What we Recommend Implement explicit business limits using assert statements to check transfer amounts against predefined thresholds. Add daily/weekly transfer limits using maps to track user activity over time, and implement additional checks for special addresses or conditions based on business requirements.

        ;; File: contracts/external/sbtc.clar
(define-public (transfer (amount uint) (sender principal) (recipient principal) (memo

Response

This is devnet only contract not intended for testnet/mainnet deployment.

Action: no action


[43] Access Control

  • critical

Ensure Least Privilege Principle in Smart Contract Access Control

How it can fail The 'execute' function is defined as public without proper access control checks at its entry point. While internal functions implement 'is-dao-or-extension' checks, the main function remains unprotected. This allows any unauthorized user to call the function and perform critical administrative operations including: enabling extensions, setting team members (core and executive), configuring markets, minting tokens, initializing governance structures, setting resolution agents, and configuring DAO settings. The function accepts a 'sender' parameter but fails to validate it against tx-sender, creating a significant security vulnerability.

What we Recommend Implement explicit access control at the beginning of the execute function using one of these approaches: 1. Add authorization check using (asserts! (is-eq tx-sender ) (err u403)) 2. Implement a contract owner check like (try! (is-contract-owner))

3. Consider implementing a more sophisticated role-based permission system 4. Add a one-time initialization guard using a state variable to prevent multiple calls to this function 5. Consider requiring multi-signature approval for sensitive operations

;; File: contracts/proposals/devnet/bdp000-bootstrap.clar
(define-public (execute (sender principal)) ...

Response

This is a bootstrapping proposal passed to the construct method of the DAO. It can only be called once and is guarded by the deployer. Its considered to be part of the DAO deployment.

Action: no action


[46] Access Control

  • critical

Ensure Least Privilege Principle in Smart Contract Access Control

How it can fail The execute function has a critical access control vulnerability. While it accepts a sender parameter, it fails to validate it against the tx-sender, allowing any user to call this function and advance the IDO stage. Although the token-sale contract's advance-ido-stage function includes an is-dao-or-extension check, if this contract is registered as an extension, the intended access restrictions could be bypassed, creating a security risk

What we Recommend Implement proper access control in the execute function by: 1) Adding explicit validation of tx-sender against the sender parameter using (asserts! (is-eq tx-sender sender) err-unauthorized), 2) Implementing role-based access control to restrict function access to authorized principals only, 3) Consider removing the sender parameter if it's not needed for authorization, and 4) Ensure extension contracts maintain consistent access control with the contracts they interact with

Response

General comment on proposals

The sender is passed to the proposal execute method according to the proposal trait because this contract can only effect the DAO if execute is called by the main dao contract. Therefore having sender in the trait gives the proposal the possibility of passing tx-sender to the DAO internals. In practice this feature is seldom needed.

This is a devnet only contract.

Action: no action

[47] Access Control

  • critical

Secure storage of access control attributes in trusted contracts

Response

See General Comment on proposals above.

As above - this is a devnet only contract.

Action: no action

[48] Access Control

  • critical

Secure storage of access control attributes in trusted contracts

Response

See General Comment on proposals above.

As above - this is a devnet only contract.

Action: no action

[49] Access Control

  • critical

Secure storage of access control attributes in trusted contracts

Response

See General Comment on proposals above.

As above - this is a devnet only contract.

Action: no action

[24] Access Control

  • high

Identification of External Libraries Critical for Smart Contract Operation

How it can fail The executive-action function has a security vulnerability where it accepts any contract implementing proposal-trait> without proper source verification. While it includes checks for executive team membership and signal requirements, it lacks validation of the proposal contract's authenticity and trusted source before execution

What we Recommend Implement an allowlist of trusted proposal contract principals. Before execution, verify that the contract-of proposal is in this allowlist. Alternatively, implement a proposal registration process where new proposal contracts must be approved by governance before they can be executed

;; File: contracts/extensions/bme004-0-core-execute.clar
(define-public (executive-action (proposal  proposal-trait>))

Response

These concerns are legitimate but miss the context inwhich the DAO operates. Firstly the DAO has strict controls on;

Its correct that proposals can execute arbitrary code but this code can only be executed if either;

  • the core team executes via emergency execution poweres - as is the case here

  • the DAO community agree to execute the proposal which as things stand can only be proposed by core DAO team members

The first measure also has a sunset period which prevents the core team from executing a proposal directly once the DAO is deemed to be stable.

Proposal submission is a subject of intense debate in the DAO community. The rules here are fairly strict but can evolve through proposal voting.

Action: no action


[24] Access Control

  • medium

Identification of External Libraries Critical for Smart Contract Operation

        ;; File: contracts/bigmarket-dao.clar
(define-public (construct (proposal  proposal-trait>))
        (let ((sender tx-sender))
                (asserts! (is-eq sender (var-get executive)) err-unauthorised)
                (var-set executive (as-contract tx-sender))
                (as-contract (execute proposal sender))
) )

Response

See general comments on proposals. Proposals contract code is intended to be community scrutinised and has to pass through the propose gateway before it can be executed.

In this case the proposal is the bootstrap proposal called in the construct method - this can only be called once and then only by the DAO deployer.

Action: no action


[6] Business Logic

  • medium

See also;

  • [7] Business Logic,

  • [16] Business Logic

  • [17] Business Logic

Ensure contract enforces appropriate business limits

How it can fail The bmg-burn function lacks explicit business limits on the amount of tokens that can be burned in a single transaction. While it has access control, an authorized caller could potentially burn an excessive or unreasonable amount of tokens without any constraints.

What we Recommend Implement explicit business limits by adding assertions that check if the burn amount is within acceptable thresholds. Consider adding a maximum burn limit per transaction or time period. Additionally, implement checks to ensure the burn operation aligns with the contract's business logic and economic model.

;; File: contracts/extensions/bme000-0-governance-token.clar
(define-public (bmg-burn (amount uint) (owner principal))
        (begin
                (try! (is-dao-or-extension))
) )

Response

We feel this is a governance issue and specific limits on functions could be arbitrary.

Action: no action


[32] Business Logic

  • medium

Ensure balance updates based on actual received funds, not declared amounts

File Name: contracts/extensions/bme023-0-market-predicting.clar Function Name: predict-category

What we Recommend Uncomment and implement the token transfer line in process-stake-transfer function to ensure actual funds are transferred before updating balances. The line '(try! (contract-call? token transfer transfer-amount tx-sender .bme023-0-market-predicting none))' needs to be properly implemented. Additionally, consider adding a verification step that confirms the transfer was successful before proceeding with balance updates.

Response

This comment actually only applies to the bitcoin market contract. Its not valid because the user transfers their stake in this contract by sending a bitcoin transaction independently.

Action: no action


[3] Access Control

Severity Low
File Name: contracts/bigmarket-dao.clar

Call Inside As Contract

In Clarity, when executing a contract call, the tx-sender is the contract that initiated the call. This means that if a contract calls another contract, the tx-sender of the called contract will be the calling contract. This can be exploited by a malicious contract to impersonate another contract.

Function Code

;; File: contracts/bigmarket-dao.clar
(define-public (execute (proposal  proposal-trait>) (sender principal))
        (begin
                (try! (is-self-or-extension))
                (asserts! (map-insert executed-proposals (contract-of proposal) stacks-
block-height) err-already-executed)
(print {event: "execute", proposal: proposal})
(as-contract (contract-call? proposal execute sender))

Response

Action: no action - permissible use of tx-sender in DAO code


[4] Business Logic

Severity Low
File Name: contracts/bigmarket-dao.clar

Inadequate Input Validation: Lack of Positive Validation Approach

In Clarity smart contracts, implement thorough input validation using a positive validation approach (allowlisting) for public functions (define-public). Despite Clarity's strong type system, additional checks are crucial to prevent unexpected behavior or exploitation. Utilize Clarity's built-in functions and control structures to explicitly allow only valid inputs, rather than attempting to block all invalid ones. This approach is especially important for parameters provided by external callers, ensuring contract integrity and security.

How it can fail

The execute function has insufficient validation for proposal parameters. While
access control exists through is-self-or-extension, the contract blindly trusts any
proposal implementing the proposal-trait interface without verifying its legitimacy,
making it vulnerable to malicious contract execution.

What we Recommend

Implement an allowlist mechanism for proposal contracts. Before execution, verify the
proposal contract address against a map of approved proposal contracts. This map
should be maintained by governance processes. Additionally, consider implementing
proposal-specific validation logic to ensure the proposal meets expected criteria
before execution.

Function Code

        ;; File: contracts/bigmarket-dao.clar
(define-public (execute (proposal  proposal-trait>) (sender principal))
        (begin
                (try! (is-self-or-extension))
                (asserts! (map-insert executed-proposals (contract-of proposal) stacks-
block-height) err-already-executed)

Response

Action: no action - permissible use of tx-sender in DAO code

[5] Access Control

Severity Low

See also

  • [10] Access Control

  • [11] Access Control

  • [14] Access Control

  • [15] Access Control

  • [18] Access Control

  • [19] Access Control

  • [20] Access Control

  • [21] Access Control

  • [22] Access Control

  • [27] Access Control

File Name: contracts/bigmarket-dao.clar Function Name: set-extensions-iter

Tx-Sender in Assert

In Clarity, the tx-sender keyword is used to get the principal of the current transaction. Since tx-sender can change during the execution of a contract, using it inside an assert statement can lead to unexpected behavior. Because of that, be mindful of the context in which tx-sender is used inside an assert.

Function Code

;; File: contracts/bigmarket-dao.clar
(define-private (set-extensions-iter (item {extension: principal, enabled: bool}))
        (begin
                (print {event: "extension", extension: (get extension item), enabled:
(get enabled item)})
                (map-set extensions (get extension item) (get enabled item))

Response

Action: no action - permissible use of tx-sender in DAO code


[31] Business Logic

Severity Low

File Name: contracts/extensions/bme023-0-market-bitcoin.clar Function Name: set-dao-treasury

Inadequate Input Validation: Lack of Positive Validation Approach

In Clarity smart contracts, implement thorough input validation using a positive validation approach (allowlisting) for public functions (define-public). Despite Clarity's strong type system, additional checks are crucial to prevent unexpected behavior or exploitation. Utilize Clarity's built-in functions and control structures to explicitly allow only valid inputs, rather than attempting to block all invalid ones. This approach is especially important for parameters provided by external callers, ensuring contract integrity and security.

How it can fail

What we Recommend

The set-dao-treasury function lacks proper input validation for the new-dao-treasury
parameter. While access control is implemented through is-dao-or-extension, there are
no validation checks for the new treasury address against specific criteria such as
burn address verification or allowlist membership.
Implement specific validation checks for the new-dao-treasury parameter. Consider
adding checks to ensure the address is not a burn address, is on an allowlist of
approved treasuries, or meets other business-specific requirements. Also consider
adding a check that the new address is different from the current one to prevent
unnecessary state changes.

Function Code

;; File: contracts/extensions/bme023-0-market-bitcoin.clar
(define-public (set-dao-treasury (new-dao-treasury principal))
  (begin
    (try! (is-dao-or-extension))
    (var-set dao-treasury new-dao-treasury)
    (ok true)

Response

Action: no action - only changeable via DAO and therefore there are no allow list checks for this update.


[37] Business Logic

Severity Low

File Name: contracts/extensions/bme023-0-market-scalar-pyth.clar Function Name: create-market

Inadequate Input Validation: Lack of Positive Validation Approach

In Clarity smart contracts, implement thorough input validation using a positive validation approach (allowlisting) for public functions (define-public). Despite Clarity's strong type system, additional checks are crucial to prevent unexpected behavior or exploitation. Utilize Clarity's built-in functions and control structures to explicitly allow only valid inputs, rather than attempting to block all invalid ones. This approach is especially important for parameters provided by external callers, ensuring contract integrity and security.

How it can fail

What we Recommend

The create-market function has insufficient input validation. While it performs basic
checks on parameters like fee-bips, market-duration, and cool-down-period, it lacks
validation for critical elements such as categories structure (beyond length>1),
treasury principal validity, and content validation for market-data-hash and price-
feed-id.
Implement comprehensive input validation including: verification of categories having
exactly 10 elements with min/max value checks, treasury principal validation, and
content validation for market-data-hash and price-feed-id. Consider implementing
allow-listing for critical parameters and add semantic validation to ensure parameter
values are contextually appropriate.

Function Code

        ;; File: contracts/extensions/bme023-0-market-scalar-pyth.clar
(define-public (create-market
  (categories (list 10 {min: uint, max: uint}))

Response

Action: no action - we agree validating price feeds is a good idea but is difficult in practice and would prevent supporting new feeds as they become available via Pyth oracles on Stacks!

0
Subscribe to my newsletter

Read articles from mike cohen directly inside your inbox. Subscribe to the newsletter, and don't miss out.

Written by

mike cohen
mike cohen