Underflow Vulnerability in transferFrom Function

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

The transferFrom function in the contract is susceptible to an underflow vulnerability when the allowance (allowed) is zero, and a user attempts to transfer a non-zero amount. This vulnerability can lead to an underflow in the calculation of the new allowance (newAllowance), resulting in an unintended large positive value. This, in turn, allows users to perform unauthorized transfers, bypassing the intended allowance mechanism.

for a non allowed user, allowance is zero, a transfer of any non-zero amount can trigger an underflow. The subtraction operation (newAllowance = allowed - amount) results in newAllowance becoming an excessively large positive number.

Unintended Transfer: With the large newAllowance, the user can make unauthorized transfers of substantial amounts, bypassing the intended allowance mechanism.

Proof of Concept

function transferFrom(
address from,
address to,
uint256 amount
) external virtual nonReentrant returns (bool) {
uint256 allowed = allowance[from][msg.sender]; // this would be 0

// Saves gas for unlimited approvals.
if (allowed != type(uint256).max) { //check pass as 0 != (uint256).max
  uint256 newAllowance = allowed - amount; // underflow
  _approve(from, msg.sender, newAllowance);
}

_transfer(from, to, amount);

return true;
}

Tools Used

Manual review

Recommended Mitigation Steps

Bounds Checking: Add bounds checking to ensure that the newAllowance calculation does not result in an underflow. If allowed is zero, do not perform the subtraction.

Assessed type

Under/Overflow

[c4-pre-sort]

minhquanym marked the issue as low quality report

[minhquanym]

Invalid

[c4-judge]

MarioPoneder marked the issue as unsatisfactory: Invalid