code-423n4 / 2023-10-wildcat-findings

12 stars 9 forks source link

The operation of the approve function in the WildcatMarketToken.sol file is invalid. #699

Open c4-submissions opened 10 months ago

c4-submissions commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketToken.sol#L31-L34 https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketToken.sol#L59-L62

Vulnerability details

Impact

Detailed description of the impact of this finding.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketToken.sol#L31C2-L34C4

  function approve(address spender, uint256 amount) external virtual nonReentrant returns (bool) {
    _approve(msg.sender, spender, amount);
    return true;
  }

https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketToken.sol#L59-L62

  function _approve(address approver, address spender, uint256 amount) internal virtual {
    allowance[approver][spender] = amount;
    emit Approval(approver, spender, amount);
  }

In this function, users can approve an invalid address, such as the zero address.

This operation is considered invalid since both the approver and the spender must be non-zero addresses for proper execution.

Tools Used

Recommended Mitigation Steps

To avoid the operation where users can approve to invalid addresses such as the zero address, you can add a require statement to check that both the approver and the spender are non-zero addresses. Here's an example of how you can do this in Solidity:

    function _approve(address owner, address spender, uint256 amount) internal virtual {
        require(owner != address(0), "ERC20: approve from the zero address");
        require(spender != address(0), "ERC20: approve to the zero address");
        ...
    }

Assessed type

Invalid Validation

c4-pre-sort commented 10 months ago

minhquanym marked the issue as low quality report

minhquanym commented 10 months ago

QA

c4-judge commented 10 months ago

MarioPoneder changed the severity to QA (Quality Assurance)

c4-judge commented 10 months ago

MarioPoneder marked the issue as grade-b