sherlock-audit / 2022-11-telcoin-judging

0 stars 0 forks source link

imare - Telcoin token address can become stale #58

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

imare

medium

Telcoin token address can become stale

Summary

In an event of upgrading the StakingModule contract address used for Telcoin token operations can become stale.

Vulnerability Detail

StakingModule upgrade can change the address used for the Telcoin token by the initializer

https://github.com/sherlock-audit/2022-11-telcoin/blob/main/contracts/StakingModule.sol#L60-L61

Inside SimplePlugin and FeeBuyback contract this address is fixed in the constructor.

https://github.com/sherlock-audit/2022-11-telcoin/blob/main/contracts/SimplePlugin.sol#L39-L41

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

Impact

Using stale address is never a good best practice. The main problem is that just redeploying the problematic contracts is not the easiest solution in case when we have already accumulated user claims. In this case a migration is needed which as stated in the comments is not a cheap operation doing it for every user account.

https://github.com/sherlock-audit/2022-11-telcoin/blob/main/contracts/StakingModule.sol#L454-L456

Also the solution proposed is for the migration of the StakingModule contract and not the mentioned ones.

Code Snippet

Tool used

Manual Review

Recommendation

The easiest solution would be to get the correct address trough the StakingModule contract before any token operation like is done in this line:

https://github.com/sherlock-audit/2022-11-telcoin/blob/main/contracts/SimplePlugin.sol#L41

amshirif commented 1 year ago

Simply updating the token address if Telcoin were to become "stale" would abandon all TEL it currently holds, and assuming TEL has become worthless. Migration capabilities are possible with current contract and the cost associated with a migration is not desired, but is acceptable.