search

sherlock-audit / 2022-11-telcoin-judging

0 stars 0 forks source link

bin2chen - submit() does not check the useless msg.value, which may cause the loss of funds #67

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

bin2chen

medium

submit() does not check the useless msg.value, which may cause the loss of funds

Summary

FeeBuyback#submit() When token == address(_telcoin), msg.value is not used, but there is no check msg.value==0, so if it is transferred by mistake, the funds will be lost.

Vulnerability Detail

contract FeeBuyback is IFeeBuyback, TieredOwnership {
..
 function submit(address wallet, bytes memory walletData, address token, address recipient, uint256 amount, bytes memory swapData) external override payable onlyOwner() returns (bool) {

    if (token == address(_telcoin)) {
      /***@audit no check msg.value==0, if it is transferred by mistake ***/
      _telcoin.transferFrom(_safe, address(this), amount);
      _telcoin.approve(address(_referral), _telcoin.balanceOf(address(this)));
      require(_referral.increaseClaimableBy(recipient, _telcoin.balanceOf(address(this))), "FeeBuyback: balance was not adjusted");
      return true;
    }

Impact

if it is transferred by mistake, the funds will be lost.

Code Snippet

https://github.com/sherlock-audit/2022-11-telcoin/blob/main/contracts/fee-buyback/FeeBuyback.sol#L61-L66

Tool used

Manual Review

Recommendation

1: if token == address(_telcoin) require msg.value==0

  1. token != address(_telcoin) , Excess value returned at the end 
function submit(address wallet, bytes memory walletData, address token, address recipient, uint256 amount, bytes memory swapData) external override payable onlyOwner() returns (bool) {
...

    if (token == address(_telcoin)) {
+   require(msg.value==0,"bad msg.value");
      _telcoin.transferFrom(_safe, address(this), amount);
      _telcoin.approve(address(_referral), _telcoin.balanceOf(address(this)));
      require(_referral.increaseClaimableBy(recipient, _telcoin.balanceOf(address(this))), "FeeBuyback: balance was not adjusted");
      return true;
    }

....
    (bool swapResult,) = _aggregator.call{value: msg.value}(swapData);
    require(swapResult, "FeeBuyback: swap transaction failed");
    _telcoin.approve(address(_referral), _telcoin.balanceOf(address(this)));
    require(_referral.increaseClaimableBy(recipient, _telcoin.balanceOf(address(this))), "FeeBuyback: balance was not adjusted");

+   if (address(this).balance >0){ // Excess value returned
+      msg.sender.call{value:address(this).balance}("");
+    }
    return true;
  }