Eggstravaganza
First Flight #37
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Invalid

Unused Return Value from External Call May Cause Logic Inconsistencies

markomarko

Summary

In the EggHuntGame::searchForEgg function, the return value from the external call to eggNFT::mintEgg is ignored. The function is defined to return a bool, indicating whether the minting was successful. However, this value is not used, and the contract proceeds to update state variables such as eggCounter and eggsFound[msg.sender] regardless of the minting outcome. Ignoring return values from external contract calls can lead to inconsistencies in contract state and potential failure to meet functional expectations.

Vulnerability Details

Proof of Code:
Place the following test into EggHuntGameTest.t.sol.

interface IEggNFT {
function mintEgg(address to, uint256 tokenId) external returns (bool);
}
.
.
.
contract EggGameTest is Test {
EggstravaganzaNFT nft;
EggVault vault;
EggHuntGame game;
address owner;
address alice;
address bob;
address mockNFT = address(0x3);
​
error OwnableUnauthorizedAccount(address account);
.
.
.
function test_UncheckedReturnValue_ShouldNotUpdateStateIfMintFails() public {
EggHuntGame exploitableGame = new EggHuntGame(mockNFT, address(vault));
​
vm.mockCall(
mockNFT,
abi.encodeWithSelector(IEggNFT.mintEgg.selector, alice, 1),
abi.encode(false)
);
​
exploitableGame.setEggFindThreshold(100);
vm.prank(owner);
exploitableGame.startGame(100);
​
vm.warp(block.timestamp + 10);
vm.prank(alice);
exploitableGame.searchForEgg();
​
// We expect eggCounter to increment, even though mint failed
// state was updated despite mint failure
assertEq(exploitableGame.eggCounter(), 1);
assertEq(exploitableGame.eggsFound(alice), 1);
}
}

Impact

If eggNFT::mintEgg fails and returns false, the contract will still increment eggCounter and increase the eggsFound count, even though the NFT was not successfully minted. This creates a mismatch between the recorded egg count and the actual minted NFTs, potentially leading to incorrect user balances, broken logic in later parts of the system, or even exploitable conditions if those inconsistencies are used as assumptions elsewhere in the contract.

Tools Used

  1. Foundry

  2. Aderyn

  3. Slither

Recommendations

Always check the return value of external calls that indicate success or failure. In this case, ensure the mintEgg call returns true.

function searchForEgg() external {
require(gameActive, "Game not active");
require(block.timestamp >= startTime, "Game not started yet");
require(block.timestamp <= endTime, "Game ended");
​
// Pseudo-random number generation (for demonstration purposes only)
uint256 random =
uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao, msg.sender, eggCounter))) % 100;
​
if (random < eggFindThreshold) {
eggCounter++;
eggsFound[msg.sender] += 1;
- eggNFT.mintEgg(msg.sender, eggCounter);
+ bool success = eggNFT.mintEgg(msg.sender, eggCounter);
+ assert(success);
emit EggFound(msg.sender, eggCounter, eggsFound[msg.sender]);
}
}
Updates

Lead Judging Commences

m3dython Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Unused return value

Returns a boolean value that isn't utilized by its caller

Support

FAQs

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

Give us feedback!