Summary
The transferErcTokens function in the TokenDivider contract performs arithmetic operations on token balances without proper overflow checks. Despite Solidity 0.8.x's built-in overflow protection, the logic for balance updates could still be vulnerable to certain edge cases and potential arithmetic overflows.
Vulnerability Detail
Current implementation:
function transferErcTokens(address nftAddress, address to, uint256 amount) external {
balances[msg.sender][tokenInfo.erc20Address] -= amount;
balances[to][tokenInfo.erc20Address] += amount;
IERC20(tokenInfo.erc20Address).transferFrom(msg.sender, to, amount);
}
The vulnerability exists in two areas:
No maximum balance validation
No overflow check during balance addition
No underflow protection for balance subtraction
Impact
This vulnerability could result in:
Token balance corruption
Unlimited token minting through overflow
Loss of user funds
Inconsistent state between actual ERC20 balances and internal balances
Potential economic attacks
Tool Used
Foundry
Proof of Concept
contract BalanceOverflowExploit {
TokenDivider target;
constructor(address _target) {
target = TokenDivider(_target);
}
function exploitOverflow(address nftAddress) external {
uint256 maxBalance = type(uint256).max;
target.transferErcTokens(
nftAddress,
address(this),
maxBalance - 1
);
target.transferErcTokens(
nftAddress,
address(this),
2
);
}
}
contract BalanceTest {
function demonstrateOverflow(
TokenDivider divider,
address nftAddress
) public {
uint256 largeAmount = type(uint256).max;
try divider.transferErcTokens(
nftAddress,
address(this),
largeAmount
) {
revert("Should have failed");
} catch Error(string memory reason) {
require(
keccak256(bytes(reason)) == keccak256(bytes("Overflow")),
"Wrong error"
);
}
}
}
Recommended Mitigation Steps
Implement Safe Math Checks
function _updateBalancesSafely(
address from,
address to,
address tokenAddress,
uint256 amount
) private {
uint256 senderBalance = balances[from][tokenAddress];
require(senderBalance >= amount, "Insufficient balance");
uint256 receiverBalance = balances[to][tokenAddress];
require(
receiverBalance + amount >= receiverBalance,
"Balance overflow"
);
balances[from][tokenAddress] = senderBalance - amount;
balances[to][tokenAddress] = receiverBalance + amount;
}
Add Balance Limits
contract TokenDivider {
uint256 public constant MAX_BALANCE = type(uint128).max;
function _validateBalance(uint256 balance, uint256 amount) private pure {
require(balance + amount <= MAX_BALANCE, "Balance limit exceeded");
}
}
Complete Implementation Example
function transferErcTokens(
address nftAddress,
address to,
uint256 amount
) external nonReentrant {
_validateTransferInputs(nftAddress, to, amount);
ERC20Info memory tokenInfo = _validateTokenInfo(nftAddress);
_updateBalancesSafely(
msg.sender,
to,
tokenInfo.erc20Address,
amount
);
bool success = IERC20(tokenInfo.erc20Address).transferFrom(
msg.sender,
to,
amount
);
require(success, "Transfer failed");
emit TokensTransferred(
msg.sender,
to,
tokenInfo.erc20Address,
amount,
block.timestamp
);
}
function _validateTransferInputs(
address nftAddress,
address to,
uint256 amount
) private view {
require(amount > 0, "Invalid amount");
require(amount <= MAX_BALANCE, "Amount exceeds limit");
require(to != address(0), "Invalid recipient");
require(nftAddress != address(0), "Invalid NFT address");
}
Add Balance Reconciliation
function reconcileBalances(
address tokenAddress
) external onlyOwner {
uint256 contractBalance = IERC20(tokenAddress).balanceOf(address(this));
uint256 recordedBalance = totalRecordedBalance[tokenAddress];
require(
contractBalance >= recordedBalance,
"Balance mismatch"
);
}
Additional Recommendations
Implement Balance Tracking
mapping(address => uint256) public totalRecordedBalance;
function _trackBalance(address token, uint256 amount, bool isAdd) private {
if (isAdd) {
totalRecordedBalance[token] += amount;
} else {
totalRecordedBalance[token] -= amount;
}
}
Add Emergency Pause
modifier whenNotPaused() {
require(!paused, "Transfers paused");
_;
}
function emergencyPause() external onlyOwner {
paused = true;
emit TransfersPaused(block.timestamp);
}
