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

safeTransferFrom not being used properly

Shitzu

Summary

The vulnerability in the KittyVault contract arises from the potential misuse of the safeTransferFrom function, which allows for the transfer of tokens from a user's account to the vault. Without proper access control, such as the onlyPool modifier, unauthorized addresses could call functions like executeDepawsit or executeWhiskdrawal, leading to unauthorized transfers and potential loss of user funds. Additionally, if users inadvertently approve malicious contracts, these contracts could exploit the approval to transfer tokens without user consent. Ensuring strict access control, user awareness of approvals, and adherence to approved allowances are critical to mitigating these risks.

Vulnerability Details

  1. Unauthorized Access:

    • Scenario: Without the onlyPool modifier, any address could call executeDepawsit or executeWhiskdrawal.

    • Exploit: An attacker could call executeWhiskdrawal to withdraw tokens from the vault to their own address, draining user funds.

  2. Approval Exploitation:

    • Scenario: A user mistakenly approves a malicious contract to spend their tokens.

    • Exploit: The malicious contract calls safeTransferFrom to transfer tokens from the user's account to an attacker-controlled address, resulting in loss of user funds.

  3. Exceeding Allowances:

    • Scenario: The contract attempts to transfer more tokens than the user has approved.

    • Exploit: The transaction fails, potentially causing disruptions in the contract's operations and locking user funds within the contract.

Impact

  1. Unauthorized Transfers:

    • Risk: Without proper access control, any address could potentially call executeDepawsit or executeWhiskdrawal, leading to unauthorized transfers of tokens. This could result in significant financial loss for users.

    • Impact: Users' funds could be drained from their accounts without their consent, eroding trust in the contract and causing potential legal and reputational damage.

  2. Approval Exploitation:

    • Risk: If an attacker gains approval to spend tokens on behalf of a user, they could exploit functions like safeTransferFrom to transfer tokens from the user's account to the vault or another address.

    • Impact: Users could lose their tokens if they inadvertently approve malicious contracts or if their approvals are misused. This could lead to a loss of user funds and trust in the platform.

  3. Exceeding Allowance:

    • Risk: Without proper checks, the contract could attempt to transfer more tokens than the user has approved, leading to failed transactions and potential contract malfunctions.

    • Impact: This could disrupt the normal operation of the contract, causing user transactions to fail and potentially locking funds within the contract.

Tools Used

  • Audit Wizard

  • Read the code

Recommendations

  1. Access Control: Ensure only authorized entities can call functions that use safeTransferFrom.

  2. Approval Management: Implement checks to ensure that approvals are only granted to trusted contracts.

  3. Allowance Checks: Verify that the approved allowance is not exceeded during transfers.

Enhanced Implementation:

  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.