Figure out a way to prevent direct calls to the plugin

[PaulRBerg]

Problem

As pointed out by @andreivladbrg in this comment, and as further explained by me in this reply, there is a low-risk security problem in our current proxy plugin implementation.

The problem is that anyone can call the onStreamCanceled implementation on the plugin, like this:

uint128 senderAmount = 10e18;
uint128 recipientAmount = 0;
ISablierV2LockupSender(address(proxy)).onStreamCanceled(lockupContract, streamId, recipient, senderAmount, recipientAmount);

And thus, anyone can withdraw any amount of ERC-20 assets from the proxy contract to the proxy owner:

https://github.com/sablierhq/v2-periphery/blob/e62b768/src/SablierV2ProxyPlugin.sol#L53

This is a low-risk issue because, generally, there is no incentive to trigger random ERC-20 transfers on behalf of other accounts, but there may be cases when this is an undesirable state of affairs:

1. Adversarial contexts. The proxy contract may perform a flash loan, and a competing flash borrower who sees their tx in the mempool could block it by withdrawing the assets to their EOA.
2. Griefing: deliberately blocking someone's proxy contract transactions for no particular benefit.
3. Tax implications. Withdrawing assets from the proxy contract might be interpreted as a taxable event.

Potential Solution

The lockup address is a user-provided function parameter:

https://github.com/sablierhq/v2-periphery/blob/e62b768026d5df52a3df340cd0df108bf455629e/src/SablierV2ProxyPlugin.sol#L37

This makes it difficult to mitigate the problem. The only solution I can think of now is to add an allowlist of Sablier contract addresses in SablierV2ProxyPlugin and cross-check lockup against it. However, implementing this would also require us to inherit from Adminable and write an admin-gated function for listing new contracts, i.e., addSablierContract, which would be demanding.

[PaulRBerg]

There was a misconception on my part when I wrote this:

implementing this would also require us to inherit from Adminable and write an admin-gated function for listing new contracts

We can't inherit from Adminable in the proxy plugin because the plugin is executed within the proxy's context, which requires us to use the proxy's storage layout and values.

The only way to prevent direct calls to the plugin is to implement a chain log contract, which is linked to the proxy plugin via an immutable reference.