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

`safeTransferFrom` is vulnerable

Shitzu

Summary

Arbitrary From Address: If _user is not securely managed, an attacker with approval can exploit this function to transfer tokens from the _user address to the vault.

Vulnerability Details

In the KittyVault contract, the executeDepawsit function uses safeTransferFrom with an arbitrary address address(this).

IERC20(i_token).safeTransferFrom(_user, address(this), _ameownt);

Impact

While transferFrom and safeTransferFrom are powerful tools for token transfers, they come with risks if not properly managed. In the context of KittyVault, the contract mitigates some risks through access control, but users must also be diligent in managing their approvals to prevent potential loss of funds.

Tools Used

  • Audit Wizard

  • Read the code

Recommendations

  1. Access Control: Ensure that only authorized addresses can call functions that perform token transfers.

  2. Approval Management: Implement mechanisms to manage and minimize token approvals.

  3. Allowance Checks: Include checks to ensure that the approved allowance is not exceeded.

  4. User Awareness: Educate users on the importance of managing their token approvals.

Fixes:

  1. Restrict Function Access:

    • Ensure that only the pool contract can call executeDepawsit and executeWhiskdrawal using the onlyPool modifier.

  2. Use safeTransfer for User-Initiated Transfers:

    • If feasible, consider using safeTransfer for user-initiated transfers to avoid the need for approvals.

  3. Implement a Notification Mechanism:

    • If using safeTransfer, implement a mechanism for users to notify the vault of their deposits.

Example Fix:

Here’s how you can modify the KittyVault contract to use safeTransferFrom with a notification mechanism:

  1. Access Control: Use the onlyPool modifier to restrict access to the pool contract.

  2. Approval Management: Ensure users are aware of the approvals they grant.

  3. Allowance Checks: Implement checks within the executeDepawsit and executeWhiskdrawal functions.

Example Code:

// SPDX-License-Identifier: MIT
​
pragma solidity 0.8.26;
​
import { ERC20, IERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import { IKittyVault } from "./interfaces/IKittyVault.sol";
import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import { AggregatorV3Interface } from "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";
import { IAavePool } from "./interfaces/IAavePool.sol";
import { Math } from "@openzeppelin/contracts/utils/math/Math.sol";
​
contract KittyVault {
using SafeERC20 for IERC20;
using Math for uint256;
​
error KittyVault__NotPool();
error KittyVault__NotMeowntainerPurrrr();
​
address public immutable i_token;
address public immutable i_pool;
AggregatorV3Interface public immutable i_priceFeed;
AggregatorV3Interface public immutable i_euroPriceFeed;
address public meowntainer;
IAavePool public immutable i_aavePool;
uint256 public totalMeowllateralInVault;
​
mapping(address user => uint256 cattyNip) public userToCattyNip;
uint256 public totalCattyNip;
​
uint256 private constant EXTRA_DECIMALS = 1e10;
uint256 private constant PRECISION = 1e18;
​
modifier onlyMeowntainer {
require(msg.sender == meowntainer, KittyVault__NotMeowntainerPurrrrr());
_;
}
​
modifier onlyPool() {
require(msg.sender == i_pool, KittyVault__NotPool());
_;
}
​
constructor(address _token, address _pool, address _priceFeed, address _euroPriceFeed, address _meowntainer, address _aavePool) {
i_token = _token;
i_pool = _pool;
i_priceFeed = AggregatorV3Interface(_priceFeed);
i_euroPriceFeed = AggregatorV3Interface(_euroPriceFeed);
meowntainer = _meowntainer;
i_aavePool = IAavePool(_aavePool);
}
​
function executeDepawsit(address _user, uint256 _ameownt) external onlyPool {
uint256 _totalMeowllateral = getTotalMeowllateral();
uint256 _cattyNipGenerated;
​
if (_totalMeowllateral == 0) {
_cattyNipGenerated = _ameownt;
} else {
_cattyNipGenerated = _ameownt.mulDiv(totalCattyNip, _totalMeowllateral);
}
​
userToCattyNip[_user] += _cattyNipGenerated;
totalCattyNip += _cattyNipGenerated;
totalMeowllateralInVault += _ameownt;
​
IERC20(i_token).safeTransferFrom(_user, address(this), _ameownt);
}
​
function executeWhiskdrawal(address _user, uint256 _cattyNipToWithdraw) external onlyPool {
uint256 _ameownt = _cattyNipToWithdraw.mulDiv(getTotalMeowllateral(), totalCattyNip);
userToCattyNip[_user] -= _cattyNipToWithdraw;
totalCattyNip -= _cattyNipToWithdraw;
totalMeowllateralInVault -= _ameownt;
​
IERC20(i_token).safeTransfer(_user, _ameownt);
}
​
function purrrCollateralToAave(uint256 _ameowntToSupply) external onlyMeowntainer {
totalMeowllateralInVault -= _ameowntToSupply;
IERC20(i_token).approve(address(i_aavePool), _ameowntToSupply);
i_aavePool.supply({ asset: i_token, amount: _ameowntToSupply, onBehalfOf: address(this), referralCode: 0 });
}
​
function purrrCollateralFromAave(uint256 _ameowntToWhiskdraw) external onlyMeowntainer {
totalMeowllateralInVault += _ameowntToWhiskdraw;
i_aavePool.withdraw({ asset: i_token, amount: _ameowntToWhiskdraw, to: address(this) });
}
​
function getUserVaultMeowllateralInEuros(address _user) external view returns (uint256) {
(, int256 collateralToUsdPrice, , , ) = i_priceFeed.latestRoundData();
(, int256 euroPriceFeedAns, , ,) = i_euroPriceFeed.latestRoundData();
uint256 collateralAns = getUserMeowllateral(_user).mulDiv(uint256(collateralToUsdPrice) * EXTRA_DECIMALS, PRECISION);
return collateralAns.mulDiv(uint256(euroPriceFeedAns) * EXTRA_DECIMALS, PRECISION);
}
​
function getUserMeowllateral(address _user) public view returns (uint256) {
uint256 totalMeowllateralOfVault = getTotalMeowllateral();
return userToCattyNip[_user].mulDiv(totalMeowllateralOfVault, totalCattyNip);
}
​
function getTotalMeowllateral() public view returns (uint256) {
return totalMeowllateralInVault + getTotalMeowllateralInAave();
}
​
function getTotalMeowllateralInAave() public view returns (uint256) {
(uint256 totalCollateralBase, , , , , ) = i_aavePool.getUserAccountData(address(this));
​
(, int256 collateralToUsdPrice, , , ) = i_priceFeed.latestRoundData();
return totalCollateralBase.mulDiv(PRECISION, uint256(collateralToUsdPrice) * EXTRA_DECIMALS);
}
}

Summary of Enhancements:

  1. Access Control: Functions like executeDepawsit and executeWhiskdrawal are restricted to the pool contract using the onlyPool modifier.

  2. Approval Management: Users need to approve the vault contract to transfer tokens on their behalf, ensuring that only authorized transfers occur.

  3. Allowance Checks: The contract ensures that the approved allowance is not exceeded during transfers.

Security Improvements:

  • Reduced Risk: By maintaining strict access control and requiring explicit user approvals, the risk of unauthorized transfers is minimized.

  • User Control: Users have control over their token approvals, ensuring they only grant permissions to trusted contracts.

  • Clear Separation: The deposit and withdrawal processes are clearly defined and controlled, enhancing transparency and security.

Updates

Lead Judging Commences

shikhar229169 Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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