Certain tokens stuck in PA1D

[code423n4]

Lines of code

https://github.com/code-423n4/2022-10-holograph/blob/24bc4d8dfeb6e4328d2c6291d20553b1d3eff00b/src/enforcer/PA1D.sol#L317

Vulnerability details

Impact

PA1D uses require(erc20.transfer(addresses[i], sending), "PA1D: Couldn't transfer token"); to transfer the token to the recipient. This will not work for tokens that do not return a boolean on success (USDT, BNB, OMG). While it is known as a low severity issue that unsafe ERC20 methods are used there, the severity in this specific case is higher: PA1D receives royalties, so it is likely that it will receive such a token (especially USDT) at some point. The transfer to the contract will succeed without any problems. However, nobody will be able to transfer the tokens out, meaning they will be stuck forever, leading to a loss of funds.

Proof Of Concept

Let's say an NFT is sold in USDT with 10% royalties. These royalties cannot be retrieved by the configured payout addresses, because the transfer will revert.

Recommended Mitigation Steps

Use OpenZeppelin's SafeERC20 library.

[gzeoneth]

Duplicate of #456