code-423n4 / 2023-05-ajna-findings

2 stars 0 forks source link

Logic error in `_transferAjnaRewards` function #498

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/RewardsManager.sol#L811-L820

Vulnerability details

Context:


ajna-core/src/RewardsManager.sol:
  836       */
  837:     function _transferAjnaRewards(uint256 rewardsEarned_) internal {
  838:         // check that rewards earned isn't greater than remaining balance
  839:         // if remaining balance is greater, set to remaining balance
  840:         uint256 ajnaBalance = IERC20(ajnaToken).balanceOf(address(this));
  841:         if (rewardsEarned_ > ajnaBalance) rewardsEarned_ = ajnaBalance;
  842: 
  843:         if (rewardsEarned_ != 0) {
  844:             // transfer rewards to sender
  845:             IERC20(ajnaToken).safeTransfer(msg.sender, rewardsEarned_);
  846:         }
  847:     }

Description: The _transferAjnaRewards function is responsible for sending to msg.sender the amount sent by the functions that calculate the earned Ajna token amount.

First control; "If there are less tokens on the contract than the reward" If there are less tokens on the contract than the reward, it will send the same amount of tokens on the contract;

 840:         uint256 ajnaBalance = IERC20(ajnaToken).balanceOf(address(this));
 841:         if (rewardsEarned_ > ajnaBalance) rewardsEarned_ = ajnaBalance;

However, with mathematical calculations and operation, the rewards should be fully received, if there is a reward that cannot be fully received, the process should be reverted, it should be ensured that it is not missing

Recommendation: Architecturally, this function should not be used to send awards, awards should be sent directly

Assessed type

Invalid Validation

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #361

c4-judge commented 1 year ago

Picodes changed the severity to 3 (High Risk)

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory