_payoutToken() breaks if tokenAddress is USDT - for Ethereum contracts.

[code423n4]

Lines of code

https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/contracts/enforcer/PA1D.sol#L416

Vulnerability details

If USDT is used for a sale at some point - either through a direct sale on the NFT collection, or sent to the collection from a marketplace sale - it will remain in the contract, as getTokenPayout(address(USDT)) calls systematically revert:

on Ethereum, USDT.transfer does not return a bool, meaning this check will always fail.

Impact

enforcer/PA1D does not have any other ERC20 withdrawal function. While enforcer/PA1D is meant to be used via delegate calls from a NFT collection contract, if the NFT contract does not have any withdrawal function either, this dust mentioned above is effectively lost.

Tools Used

Manual Analysis

Mitigation

Use OpenZeppelin's library SafeERC20.safeTransfer():
If the ERC20 token does not return any boolean upon transfer like USDT, the call still goes through. It will only revert if the ERC20.transfer call reverts or return false.

[d3e4]

Duplicate of #456

[gzeoneth]

Duplicate of #456

[ACC01ADE]

PA1D contract is upgrade-able, so issue can be fixed post-fact.