Strict Equality Comparison in Block Number Verification.

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/src/core/VaultManagerV2.sol#L143

Vulnerability details

Impact

The use of a strict equality check (==) in the VaultManagerV2.withdraw function to compare idToBlockOfLastDeposit[id] with block.number introduces a potential security vulnerability. This vulnerability arises because the strict equality operator does not perform type coercion, meaning that if the types of the operands are different, the comparison will always return false. In the context of smart contracts, this can lead to unexpected behavior, especially when dealing with block numbers, which are of type uint256. If idToBlockOfLastDeposit[id] is not explicitly cast to uint256, the comparison could fail even if the values are logically equal, potentially preventing withdrawals or other operations that rely on this check.

Proof of Concept

The code snippet provided demonstrates the use of strict equality in a critical function of a smart contract:

function withdraw(uint256, address, uint256, address)  {
    require(idToBlockOfLastDeposit[id] == block.number, "Withdrawal not allowed");
}

This code snippet is part of the withdraw function in the VaultManagerV2 contract. The require statement checks if the block number of the last deposit (idToBlockOfLastDeposit[id]) is strictly equal to the current block number (block.number). If this condition is not met, the function reverts with an error message.

Tools Used

Slither was used to identify this vulnerability.

Recommended Mitigation Steps

To mitigate the risk associated with the use of strict equality in this context, consider the following recommendations:

1. Type Coercion: Ensure that both operands are of the same type before performing the comparison. This can be achieved by explicitly casting one of the operands to the type of the other. For example, if idToBlockOfLastDeposit[id] is stored as a different type, you should cast it to uint256 before the comparison.

   require(uint256(idToBlockOfLastDeposit[id]) == block.number, "Withdrawal not allowed");

2. Use SafeMath for Arithmetic Operations: If arithmetic operations are involved in the comparison, consider using SafeMath library for arithmetic to prevent overflows and underflows.

Assessed type

Other

[c4-pre-sort]

JustDravee marked the issue as insufficient quality report

[c4-judge]

koolexcrypto marked the issue as unsatisfactory: Invalid