Lack of access control in AllowedMsgSenders contract.

[code423n4]

Lines of code

https://github.com/prepo-io/prepo-monorepo/blob/3541bc704ab185a969f300e96e2f744a572a3640/packages/prepo-shared-contracts/contracts/AllowedMsgSenders.sol#L15

Vulnerability details

Impact

setAllowedMsgSenders() lacks access control. This enables anyone to set themselves as an allowed message sender and call the send() in tokenSender.sol to transfer out any amount bypassing all the checks imposed in the hooks.

Proof of Concept

WithdrawHook.sol#L53 is the hook function that calls tokenSender.sol to reimburse fees to a user after doing the necessary checks.

Send() has a check onlyAllowedMsgSenders to restrict access to allowed senders. However, since anyone can set themselves as an allowed account, this check is bypassed.

function send(address recipient, uint256 unconvertedAmount) external override onlyAllowedMsgSenders {}.

Tools Used

Manual auditing

Recommended Mitigation Steps

If this contract was intended to be deployed by a third party and this is the reason for not having any access control, there is a lack of any internal owner-setting mechanism. Such as an address specifying the owner that can be set in the constructor when deploying.

Consider adding address public owner and a constructor for setting set value. Then add a require statement to restrict who can call setAllowedMsgSenders() to only owner.

[Picodes]

Appropriate access control is set in MintHook, RedeemHook, TokenSender

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid