First Flight #21: KittyFi
First Flight #21
Beginner FriendlyDeFiFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Invalid

Unhandled Arithmetic Overflow/Underflow in `KittyPool::burnKittyCoin` Function Leading to Unexpected Reverts

andir16

Summary

The KittyPool::burnKittyCoinfunction lacks proper input validation, causing arithmetic overflow/underflow errors. This leads to unexpected reverts when users attempt to burn more tokens than their balance, disrupting contract functionality and potentially causing significant harm.

Vulnerability Details

The KittyPool::burnKittyCoin function does not include proper checks to ensure that the burn amount does not exceed the user's balance. As a result, when a user attempts to burn more tokens than they possess, the function reverts with a panic code due to arithmetic underflow. This lack of validation allows for uncontrolled reverts, making the contract vulnerable to unexpected behavior and potential exploitation.

Quote from the solidity docs:
"Properly functioning code should never create a Panic, not even on invalid external input. If this happens, then there is a bug in your contract which you should fix." (https://docs.soliditylang.org/en/latest/control-structures.html)

Impact

  • Unexpected Reverts: Users experience unexpected transaction failures, leading to a poor user experience and potential loss of trust in the contract.

  • Disruption of Functionality: The contract's functionality can be disrupted, especially if malicious users intentionally trigger these errors.

Severity: Medium

While the issue does not directly lead to financial loss, its potential to disrupt contract functionality and the user experience warrants a medium severity rating. If further analysis reveals exploitation paths causing significant harm, the severity could be elevated to high.

Tools Used

  • Manual code review

  • Foundry for testing and reproducing the issue

Add the following to KittyFiTest.t.sol:

function test_BurningMoreKittyCoinThanUserHas() public {
uint256 toDeposit = 5 ether;
uint256 amountToMint = 20e18; // 20 KittyCoin
​
// User deposits collateral and mints KittyCoin
vm.startPrank(user);
IERC20(weth).approve(address(wethVault), toDeposit);
kittyPool.depawsitMeowllateral(weth, toDeposit);
kittyPool.meowintKittyCoin(amountToMint);
vm.stopPrank();
​
// Check initial state
assertEq(kittyPool.getKittyCoinMeownted(user), amountToMint);
​
// User tries to burn more KittyCoin than they have
uint256 toBurn = 25e18; // Attempt to burn 25 KittyCoin
vm.startPrank(user);
vm.expectRevert(abi.encodeWithSignature("Panic(uint256)", 0x11)); // Expecting arithmetic overflow/underflow
kittyPool.burnKittyCoin(user, toBurn);
vm.stopPrank();
​
// Verify state has not changed
assertEq(kittyPool.getKittyCoinMeownted(user), amountToMint);
}

Recommendations

  1. Input Validation: Add checks in the KittyPool::burnKittyCoin function to ensure the burn amount is valid and does not exceed the user's balance. Revert with meaningful error messages for better control and clarity.

function burnKittyCoin(address _onBehalfOf, uint256 _ameownt) external {
+ require(_ameownt > 0, "Burn amount must be greater than zero");
+ require(kittyCoinMeownted[_onBehalfOf] >= _ameownt, "Burn amount exceeds balance");
kittyCoinMeownted[_onBehalfOf] -= _ameownt;
i_kittyCoin.burn(msg.sender, _ameownt);
}

By implementing this recommendation, the contract will handle errors more gracefully, avoid unexpected reverts, and enhance robustness and user experience.

Updates

Lead Judging Commences

shikhar229169 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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