Skip to content

Latest commit

 

History

History
121 lines (79 loc) · 4.87 KB

025.md

File metadata and controls

121 lines (79 loc) · 4.87 KB

Soaring Pebble Meerkat

Medium

Code is breaking READme by not implementing according to the mentioned design decision

Summary

All swap functions like swap, defiToStablecoinSwap, stablecoinToDefiSwap, defiSwap calls the internal function _defiSwap to do a swap with walletData. _defiSwap then calls _feeDispersal where referral fees are sent to referrer if available, then other fees is sent to the defi safe address. If the fee token is Telcoin, then no swap is done and directly sent to safe address, or else swapped and sent to safe address via _buyBack call. Fee token can be POL, TELCOIN, or other tokens. If there are any extra tokens after swap, they are also sent to safe address without swapping.

Root Cause

Fee tokens (TELCOIN, POL, others. Sent without swap and also after swap) are sent to safe address during _feeDispersal even if safe address is not passed in swap function. Readme says, funds should not be lost in case safe address is not passed in, so code should avoid sending the fee tokens. But code is not checking if safe address is passed in or not, sending the Telcoins all the time. And this is breaking the design design decision mentioned in the readme.

Internal pre-conditions

Safe address not passed in during swap

External pre-conditions

No response

Attack Path

Please discuss any design choices you made. : https://github.com/sherlock-audit/2024-11-telcoin#q-please-discuss-any-design-choices-you-made

The issue is, Readme’s design choice says most inputs can be ignored in nearly any flow unless this would lead to a loss of funds. Also gives an example However is the value for a safe is not passed in tokens should not be able to be sent into space or the zero address.

Meaning, the protocol will perform swap with safe is not passed in so that no fees tokens are not sent out. But, regardless of safe address being passed in or not, the fee tokens are sent out. IF the fee token is not TELCOIN, then they are swapped and sent out, and any remaining non-telcoin fee token after swap, are also sent directly to safe address. these will be lost if safe address not passed in.

This is against the design decision. The code is not following the design decision mentioned in the readme, that if a safe address is not passed in, then tokens are not sent to safe or 0 address.

https://github.com/sherlock-audit/2024-11-telcoin/blob/b9c751b59e78a7123a636e31ecafc9147046f190/telcoin-audit/contracts/swap/AmirX.sol#L183-L253

contracts/swap/AmirX.sol

167:     function _defiSwap(address wallet, DefiSwap memory defi) internal {
169:         (bool walletResult, ) = wallet.call{value: 0}(defi.walletData);
170:         require(walletResult, "AmirX: wallet transaction failed");
171: 
172:  >>>    _feeDispersal(defi);
173:     }


185:     function _feeDispersal(DefiSwap memory defi) internal {
186:         // must buy into TEL
187:         if (defi.feeToken != TELCOIN)
188:             _buyBack(
189:                 defi.feeToken,
190:                 defi.aggregator,
191:   >>>           defi.defiSafe,
192:                 defi.swapData
193:             );
194: 
195:         // distribute reward
196:         if (defi.referrer != address(0) && defi.referralFee != 0) {
    ---- SNIP ----
209:         }
210:         // retain remainder
211:         if (TELCOIN.balanceOf(address(this)) > 0)
212:             TELCOIN.safeTransfer(
213:    >>>          defi.defiSafe,
214:                 TELCOIN.balanceOf(address(this))
215:             );
216:     }
217: 

226:     function _buyBack(
227:         ERC20 feeToken,
228:         address aggregator,
229:         address safe,
230:         bytes memory swapData
231:     ) internal {
232:         if (address(feeToken) == address(0)) return;
233:         if (address(feeToken) == POL) {
234:             (bool polSwap, ) = aggregator.call{value: msg.value}(swapData);
235:             require(polSwap, "AmirX: POL swap transaction failed");
236: 
237:             if (address(this).balance > 0) {
238:    >>>          (bool success, ) = safe.call{value: address(this).balance}("");
239:                 require(success, "AmirX: POL send transaction failed");
240:             }
241:         } else {
    ---- SNIP ----
248: 
249:             (bool ercSwap, ) = aggregator.call{value: 0}(swapData);
250:             require(ercSwap, "AmirX: token swap transaction failed");
251: 
252:             uint256 remainder = feeToken.balanceOf(address(this));
253:   >>>       if (remainder > 0) feeToken.safeTransfer(safe, remainder);
254:         }
255:     }

Impact

Code is not implementing readme’s design decision, so there by breaking the Readme. So medium. loss of fee tokens if safe address not passed in.

PoC

No response

Mitigation

Follow the design decision mentioned in readme