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

Lack of access control in executeDepawsit function

pro_king

Description

The executeDepawsit function currently lacks proper access control. If a user has granted approval to a malicious contract or user, the malicious entity could call executeDepawsit and pass the approved user address as _user, causing unintended token transfers. The function should ensure that the caller (msg.sender) is the owner of the tokens or has explicit permission to transfer tokens on behalf of _user.

function executeDepawsit(address _user, uint256 _ameownt) external onlyPool {
uint256 _totalMeowllateral = getTotalMeowllateral();
uint256 _cattyNipGenerated; // number of shares generated from the deposit.
​
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);
}

Impact

This issue can lead to unintended token transfers, allowing malicious entities to deposit tokens from a user's account without their explicit permission, which can result in loss of funds.

Tools Used

Manual Review

Recommendations

To mitigate this issue, add a check to ensure that the caller (msg.sender) is either the token owner or has explicit permission to transfer tokens on behalf of the user. This can be achieved by introducing access control mechanisms and using the allowance function of the ERC20 token standard to verify permissions.

Here is an updated version of the executeDepawsit function with the necessary access control checks:

function executeDepawsit(address _user, uint256 _ameownt) external onlyPool {
// Ensure the caller is the token owner or has explicit permission to transfer tokens on behalf of the user
require(msg.sender == _user || IERC20(i_token).allowance(_user, msg.sender) >= _ameownt,
"Caller must be the token owner or have explicit permission to transfer tokens");
​
uint256 _totalMeowllateral = getTotalMeowllateral();
uint256 _cattyNipGenerated; // Number of shares generated from the deposit.
​
if (_totalMeowllateral == 0) {
_cattyNipGenerated = _ameownt;
} else {
_cattyNipGenerated = _ameownt.mulDiv(totalCattyNip, _totalMeowllateral);
}
​
userToCattyNip[_user] += _cattyNipGenerated;
totalCattyNip += _cattyNipGenerated;
totalMeowllateralInVault += _ameownt;
​
// Transfer tokens from the user to the contract
IERC20(i_token).safeTransferFrom(_user, address(this), _ameownt);

}

Updates

Lead Judging Commences

shikhar229169 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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