Decent Contracts - Decent Labs

The TokenSaleV1 contract allows the protocolFeeReceiver address to be set to the zero address during initialization. In both the sellerSettle() and failed sale settlement paths, protocol fees are transferred to protocolFeeReceiver without checking if it is the zero address.

If protocolFeeReceiver is set to address(0), protocol fees will be irretrievably lost, as tokens sent to the zero address cannot be recovered. Any EOA or contract that initializes the sale with a zero address for protocolFeeReceiver will cause this loss.

Add a check in the initialize() function to ensure that protocolFeeReceiver is not the zero address.

ACKNOWLEDGED: The Decent Labs team made a business decision to acknowledge this finding and not alter the contracts, stating:

We have made a conscious decision in the codebase to not check for address(0) upon contract initialization. There are a near infinite number of incorrect addresses the contract could be initialized with, and we can of course not check for them all.

The _validateHedgeyParams function in the TokenSaleV1 contract does not check whether the cliff_ parameter is set before the start_ parameter. This allows a vesting schedule to be created where the cliff occurs before the vesting actually begins, which is illogical and could lead to confusion or unintended behavior.

For example, a sale could be initialized with start_ = 1000 and cliff_ = 900, resulting in a cliff that is 100 seconds before vesting starts. This does not align with standard vesting logic, where the cliff should always be at or after the start of vesting.

Add a validation check in _validateHedgeyParams to ensure that cliff_ is greater than or equal to start_. For example:

if (cliff_ < start_) revert InvalidCliff();

SOLVED: The Decent Labs team solved this finding in the specified commit by following the mentioned recommendation.

The documentation states the “staked token itself cannot be added as a reward token,” but _addRewardsTokens() only checks DuplicateRewardsToken() and does not prevent token == stakedToken().

The owner could call addRewardsTokens([stakedToken]) after users have staked. While _distributableRewards() special‑cases this by subtracting totalStaked, the mismatch between stated invariant and code can mislead integrators, off‑chain accounting, or frontends that assume the array never contains the staked asset.

Enforce the documented rule by modifying the _addRewardsTokens() function to revert if the token == stakedToken.

SOLVED: The Decent Labs team solved this finding in the specified commit by following the mentioned recommendation, stating:

It is intended that the staked token may also be a reward token. In the attached PR, the natspec was updated to reflect this.

Throughout the TokenSaleV1 contract, several input parameters in the initialize() function are assigned to state variables with only partial validation. While some checks are present, other critical parameters are not validated for correctness or safety. For example:

• In TokenSaleV1.initialize(), the following addresses are not checked against the zero address before assignment:

  • commitmentToken

  • saleToken

  • verifier

  • saleProceedsReceiver

  • protocolFeeReceiver

  • saleTokenHolder

  • hedgeyLockupParams.votingTokenLockupPlans

• In VerifierV1.constructor(), and the updateSigner() function, the signer_ address is not checked against the zero address before assignment.

• In VotesERC20StakedV1.initialize(), the stakedToken_ address is not checked against the zero address before assignment.

• In TokenSaleV1.initialize(), saleStartTimestamp and saleEndTimestamp are only checked for ordering and not for equality, potentially allowing a zero-length sale.

• In TokenSaleV1.initialize(), minimumCommitment and maximumCommitment are only checked for ordering and not for equality or nonzero values, potentially allowing zero commitments.

• In TokenSaleV1.initialize(), minimumTotalCommitment and maximumTotalCommitment are only checked for ordering and not for equality or nonzero values, potentially allowing zero total commitments.

• In TokenSaleV1.initialize(), saleTokenPrice is not validated to ensure it is greater than zero, which could lead to division by zero or pricing errors.

Add explicit validation to ensure that all critical input addresses are not the zero address and that numeric parameters are within reasonable, nonzero bounds. This will help prevent misconfiguration and improve the overall robustness of the contract.

ACKNOWLEDGED: The Decent Labs team made a business decision to acknowledge this finding and not alter the contracts, stating that:

We have made a conscious decision in this codebase to keep initialization parameter checks very minimal. Since there are many possible incorrect configurations, they cannot all be checked for. After reviewing the called out initialization parameters, we have to decided to leave this as is.

The contracts in scope use floating pragma version ^0.8.30 , which means that the code can be compiled by any compiler version that is greater than these versions, and less than 0.9.0.

However, it is recommended that contracts should be deployed with the same compiler version and flags used during development and testing. Locking the pragma helps to ensure that contracts do not accidentally get deployed using another pragma. For example, an outdated pragma version might introduce bugs that affect the contract system negatively.

Additionally, from Solidity versions 0.8.20 through 0.8.24, the default target EVM version is set to Shanghai, which results in the generation of bytecode that includes PUSH0 opcodes. Starting with version 0.8.25, the default EVM version shifts to Cancun, introducing new opcodes for transient storage, TSTORE and TLOAD.

In this aspect, it is crucial to select the appropriate EVM version when it's intended to deploy the contracts on networks other than the Ethereum mainnet, which may not support these opcodes. Failure to do so could lead to unsuccessful contract deployments or transaction execution issues.

Lock the pragma version to the same version used during development and testing (for example: pragma solidity 0.8.30;), and make sure to specify the target EVM version when using Solidity versions from 0.8.20 and above if deploying to chains that may not support newly introduced opcodes.

Additionally, it is crucial to stay informed about the opcode support of different chains to ensure smooth deployment and compatibility.

ACKNOWLEDGED: The Decent Labs team made a business decision to acknowledge this finding and not alter the contracts, stating that:

Since the solidity compiler is explicitly defined in the hardhat config file, a conscious decision is made when compiling with a newer solidity version. If the compiler version is incremented, and the tests all pass, then functionally it is fine to deploy with this newer version.