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 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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