Anyone can call recoverERC20, pause, unpause, and setChallengePeriod functions because of wrong "onlyOwner" implementation

Summary

Vulnerability Detail

Checking the package.json of the contract, it shows that the contract uses Openzeppelin's contract version 5 upward:

"dependencies": { "@openzeppelin/contracts": "^5.0.1", "@openzeppelin/contracts-upgradeable": "^5.0.1" }

The TelecoinDistributor.sol calls Ownable(_msgSender()) in its constructor. However, the vulnerability lies in using _msgSender() which is a function inherited by Ownable.sol from Context.sol. _msgSender() is a function that only returns the immediate sender of a call.

What this means for the onlyOwner modifier used in recoverERC20, pause, unpause, and setChallengePeriod functions is that it will not revert when called by anyone. This is as a result of calling msgSender() in the constructor. _msgSender() doesn't set any address as the owner. It only returns the immediate sender of a call.

And because of calling _msgSender() in the TelcoinDistributor.sol's constructor, functions marked with onlyOwner will only return the address calling the function and not revert.

Here's the function in the Context.sol inherited by Ownable.sol:

function _msgSender() internal view virtual returns (address) { return msg.sender; }