Pieces Protocol
First Flight #32
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

Double Zero Address Check in transferErcTokens

sauronsol

Summary

The transferErcTokens function in the TokenDivider contract performs redundant zero address validation, checking the same condition twice. This leads to unnecessary gas consumption and reduces code maintainability.

Severity: Low

Vulnerability Details

In the transferErcTokens function, there are two identical checks for the zero address:

function transferErcTokens(address nftAddress,address to, uint256 amount) external {
if(to == address(0)) {
revert TokenDivider__CantTransferToAddressZero();
}
// ... other checks ...
if(to == address(0)) { // Redundant check
revert TokenDivider__CantTransferToAddressZero();
}
// ... rest of the function
}

The second check is completely redundant as:

  1. If to is the zero address, the function will revert at the first check

  2. The value of to cannot change between the two checks

  3. The second check can never be reached if to is the zero address

Impact

The redundant check results in:

  1. Increased gas consumption (~1256 gas per transaction as demonstrated in tests)

  2. Reduced code maintainability

  3. Potential confusion for developers reviewing or maintaining the code

While this is not a critical security vulnerability, it represents an unnecessary inefficiency in the contract.

Tools Used

  • Manual code review

  • Foundry testing framework

  • Custom test suite (DoubleCheckTest.t.sol)

  • Gas measurement using Foundry's gas tracking

Recommendations

Remove the second zero address check, keeping only the first validation. The function should be refactored to:

function transferErcTokens(address nftAddress,address to, uint256 amount) external {
if(nftAddress == address(0)) {
revert TokenDivider__NftAddressIsZero();
}
​
if(to == address(0)) {
revert TokenDivider__CantTransferToAddressZero();
}
​
if(amount == 0) {
revert TokenDivider__AmountCantBeZero();
}
​
ERC20Info memory tokenInfo = nftToErc20Info[nftAddress];
​
if(balances[msg.sender][tokenInfo.erc20Address] < amount) {
revert TokenDivider__NotEnoughErc20Balance();
}
​
balances[msg.sender][tokenInfo.erc20Address] -= amount;
balances[to][tokenInfo.erc20Address] += amount;
​
emit TokensTransfered(amount, tokenInfo.erc20Address);
​
IERC20(tokenInfo.erc20Address).transferFrom(msg.sender,to, amount);
}

This change will:

  1. Reduce gas costs

  2. Improve code readability

  3. Maintain the same security guarantees

Updates

Lead Judging Commences

juan_pedro_ventu Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.