Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Vulnerabilities in Minting and Transfer Functions on ‘AccountNFT’ Contracts

juggernaut63

Summary

A vulnerability was found in the AccountNFT contract which caused failure (revert) when minting tokens. This vulnerability is caused by a lack of input validation and authorization in these functions, which can cause the contract to not function properly and potentially be misused by irresponsible parties.

Vulnerability Details

  1. mint function:
    This function does not validate whether the recipient (to) address is a null address and whether the given tokenId already exists. This causes a failure when the function is called with invalid input.

  2. _update function:
    This function fails (reverts) when the input is invalid or the authorization is not appropriate. There is insufficient validation of the to and auth parameters, which causes the contract to fail to function properly.

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/account-nft/AccountNFT.sol

Impact

  • Minting Failure

  • Abuse

Tools Used

  • Manual review

  • Foundry

Recommendations

  1. Add Validation to the mint Function

  2. Fix the _update Function

PoC

Fuzzing with Foundry:

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.25;
​
import "forge-std/Test.sol";
import "../src/account-nft/AccountNFT.sol";
​
contract AccountNFTTest is Test {
AccountNFT accountNFT;
address owner;
address addr1;
address addr2;
​
function setUp() public {
owner = address(this);
addr1 = address(1);
addr2 = address(2);
accountNFT = new AccountNFT("Account NFT", "ANFT", owner);
}
​
function testFuzzMint(address to, uint256 tokenId) public {
vm.assume(to != address(0)); // Avoid minting to the zero address
vm.prank(owner); // Set the sender to the owner
accountNFT.mint(to, tokenId);
​
assertEq(accountNFT.ownerOf(tokenId), to);
}
​
function testFuzzTransfer(address from, address to, uint256 tokenId) public {
vm.assume(to != address(0) && from != address(0)); // Avoid zero address transfers
vm.prank(owner); // Set the sender to the owner
accountNFT.mint(from, tokenId);
// Transfer token
vm.prank(from);
accountNFT.transferFrom(from, to, tokenId);
​
assertEq(accountNFT.ownerOf(tokenId), to);
}
​
function testFuzzUpdate(address to, uint256 tokenId, address auth) public {
vm.assume(to != address(0) && auth != address(0)); // Avoid zero address
vm.prank(owner); // Set the sender to the owner
accountNFT.mint(owner, tokenId);
​
// Call internal _update via a derived contract
DerivedAccountNFT derivedNFT = new DerivedAccountNFT("Derived Account NFT", "DANFT", owner);
derivedNFT.mint(owner, tokenId);
​
vm.prank(auth);
derivedNFT.update(to, tokenId, auth);
​
assertEq(derivedNFT.ownerOf(tokenId), to);
}
}
​
contract DerivedAccountNFT is AccountNFT {
constructor(string memory name, string memory symbol, address owner) AccountNFT(name, symbol, owner) {}
​
function update(address to, uint256 tokenId, address auth) external {
_update(to, tokenId, auth);
}
}

forge test --match-path test/AccountNFTTest.t.sol
[⠒] Compiling...
[⠢] Compiling 1 files with Solc 0.8.25
[⠆] Solc 0.8.25 finished in 7.00s
Compiler run successful!
proptest: Aborting shrinking after the PROPTEST_MAX_SHRINK_ITERS environment variable or ProptestConfig.max_shrink_iters iterations (set 0 t
o a large(r) value to shrink more; current configuration: 0 iterations) proptest: Aborting shrinking after the PROPTEST_MAX_SHRINK_ITERS environment variable or ProptestConfig.max_shrink_iters iterations (set 0 t
o a large(r) value to shrink more; current configuration: 0 iterations) proptest: Aborting shrinking after the PROPTEST_MAX_SHRINK_ITERS environment variable or ProptestConfig.max_shrink_iters iterations (set 0 t
o a large(r) value to shrink more; current configuration: 0 iterations)
Ran 3 tests for test/AccountNFTTest.t.sol:AccountNFTTest
[FAIL. Reason: EvmError: Revert; counterexample: calldata=0x46360e34000000000000000000000000000000000000000000000000000000000000074400000000
0000000000000000000000000000000000000000000000002f745c58 args=[0x0000000000000000000000000000000000000744, 796154968 [7.961e8]]] testFuzzMint(address,uint256) (runs: 0, μ: 0, ~: 0) [FAIL. Reason: EvmError: Revert; counterexample: calldata=0x3525b6a3000000000000000000000000000000000000000000000000000000000000074400000000
0000000000000000000000000000000000000000000000002f745c5800000000000000000000000000000000000000000000000000000000000000b7 args=[0x0000000000000000000000000000000000000744, 0x000000000000000000000000000000002F745c58, 183]] testFuzzTransfer(address,address,uint256) (runs: 0, μ: 0, ~: 0) [FAIL. Reason: EvmError: Revert; counterexample: calldata=0xe054e2ca000000000000000000000000000000000000000000000000000000000000074400000000
0000000000000000000000000000000000000000000000002f745c5800000000000000000000000000000000000000000000000000000000000000b7 args=[0x0000000000000000000000000000000000000744, 796154968 [7.961e8], 0x00000000000000000000000000000000000000b7]] testFuzzUpdate(address,uint256,address) (runs: 0, μ: 0, ~: 0) Suite result: FAILED. 0 passed; 3 failed; 0 skipped; finished in 4.60ms (9.13ms CPU time)

Ran 1 test suite in 9.76ms (4.60ms CPU time): 0 tests passed, 3 failed, 0 skipped (3 total tests)

Failing tests:
Encountered 3 failing tests in test/AccountNFTTest.t.sol:AccountNFTTest
[FAIL. Reason: EvmError: Revert; counterexample: calldata=0x46360e34000000000000000000000000000000000000000000000000000000000000074400000000
0000000000000000000000000000000000000000000000002f745c58 args=[0x0000000000000000000000000000000000000744, 796154968 [7.961e8]]] testFuzzMint(address,uint256) (runs: 0, μ: 0, ~: 0) [FAIL. Reason: EvmError: Revert; counterexample: calldata=0x3525b6a3000000000000000000000000000000000000000000000000000000000000074400000000
0000000000000000000000000000000000000000000000002f745c5800000000000000000000000000000000000000000000000000000000000000b7 args=[0x0000000000000000000000000000000000000744, 0x000000000000000000000000000000002F745c58, 183]] testFuzzTransfer(address,address,uint256) (runs: 0, μ: 0, ~: 0) [FAIL. Reason: EvmError: Revert; counterexample: calldata=0xe054e2ca000000000000000000000000000000000000000000000000000000000000074400000000
0000000000000000000000000000000000000000000000002f745c5800000000000000000000000000000000000000000000000000000000000000b7 args=[0x0000000000000000000000000000000000000744, 796154968 [7.961e8], 0x00000000000000000000000000000000000000b7]] testFuzzUpdate(address,uint256,address) (runs: 0, μ: 0, ~: 0)
Encountered a total of 3 failing tests, 0 tests succeeded

Test with Foundry:

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.25;
​
import "forge-std/Test.sol";
import "../src/account-nft/AccountNFT.sol";
​
contract AccountNFTTest is Test {
AccountNFT accountNFT;
address owner;
address addr1;
address addr2;
​
function setUp() public {
owner = address(this);
addr1 = address(1);
addr2 = address(2);
accountNFT = new AccountNFT("Account NFT", "ANFT", owner);
}
​
function testMintWithValidInput() public {
uint256 tokenId = 1;
vm.prank(owner);
emit log("Minting token with valid input");
accountNFT.mint(addr1, tokenId);
​
assertEq(accountNFT.ownerOf(tokenId), addr1);
}
​
function testMintWithInvalidAddress() public {
uint256 tokenId = 1;
vm.prank(owner);
emit log("Minting token with invalid address");
vm.expectRevert("ERC721: mint to the zero address");
accountNFT.mint(address(0), tokenId);
}
​
function testMintWithExistingTokenId() public {
uint256 tokenId = 1;
vm.prank(owner);
emit log("Minting token with valid input for the first time");
accountNFT.mint(addr1, tokenId);
​
vm.prank(owner);
emit log("Minting token with existing tokenId");
vm.expectRevert("ERC721: token already minted");
accountNFT.mint(addr2, tokenId);
}
​
function testTransferWithValidInput() public {
uint256 tokenId = 1;
vm.prank(owner);
emit log("Minting token with valid input for transfer test");
accountNFT.mint(addr1, tokenId);
​
vm.prank(addr1);
emit log("Transferring token with valid input");
accountNFT.transferFrom(addr1, addr2, tokenId);
​
assertEq(accountNFT.ownerOf(tokenId), addr2);
}
​
function testTransferWithoutOwnership() public {
uint256 tokenId = 1;
vm.prank(owner);
emit log("Minting token with valid input for transfer without ownership test");
accountNFT.mint(addr1, tokenId);
​
vm.prank(addr2);
emit log("Attempting to transfer token without ownership");
vm.expectRevert("ERC721: transfer caller is not owner nor approved");
accountNFT.transferFrom(addr1, addr2, tokenId);
}
​
function testUpdateWithValidInput() public {
uint256 tokenId = 1;
vm.prank(owner);
emit log("Minting token with valid input for update test");
accountNFT.mint(addr1, tokenId);
​
// Since _update is an internal function, we cannot call it directly.
// Instead, we test whether the contract works correctly as a whole.
// For example, we can test the notification generated by _update.
// However, without modifying the contract to expose internal functions, we cannot call it directly.
emit log("Update test - cannot call _update directly");
}
}

forge test --match-path test/AccountNFTTest.t.sol
[⠒] Compiling...
[⠢] Compiling 1 files with Solc 0.8.25
[⠆] Solc 0.8.25 finished in 6.04s
Compiler run successful!

Ran 6 tests for test/AccountNFTTest.t.sol:AccountNFTTest
[FAIL. Reason: EvmError: Revert] testMintWithExistingTokenId() (gas: 135928)
[FAIL. Reason: Error != expected error: != ERC721: mint to the zero address] testMintWithInvalidAddress() (gas: 15130)
[FAIL. Reason: EvmError: Revert] testMintWithValidInput() (gas: 135632)
[FAIL. Reason: EvmError: Revert] testTransferWithValidInput() (gas: 135908)
[FAIL. Reason: EvmError: Revert] testTransferWithoutOwnership() (gas: 136181)
[FAIL. Reason: EvmError: Revert] testUpdateWithValidInput() (gas: 135929)
Suite result: FAILED. 0 passed; 6 failed; 0 skipped; finished in 1.43ms (861.00µs CPU time)

Ran 1 test suite in 6.46ms (1.43ms CPU time): 0 tests passed, 6 failed, 0 skipped (6 total tests)

Failing tests:
Encountered 6 failing tests in test/AccountNFTTest.t.sol:AccountNFTTest
[FAIL. Reason: EvmError: Revert] testMintWithExistingTokenId() (gas: 135928)
[FAIL. Reason: Error != expected error: != ERC721: mint to the zero address] testMintWithInvalidAddress() (gas: 15130)
[FAIL. Reason: EvmError: Revert] testMintWithValidInput() (gas: 135632)
[FAIL. Reason: EvmError: Revert] testTransferWithValidInput() (gas: 135908)
[FAIL. Reason: EvmError: Revert] testTransferWithoutOwnership() (gas: 136181)
[FAIL. Reason: EvmError: Revert] testUpdateWithValidInput() (gas: 135929)

Encountered a total of 6 failing tests, 0 tests succeeded

Updates

Lead Judging Commences

inallhonesty Lead Judge
11 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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