search

code-423n4 / 2022-10-holograph-findings

1 stars 0 forks source link

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

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

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 commented 1 year ago

Duplicate of #456

gzeoneth commented 1 year ago

Duplicate of #456

ACC01ADE commented 1 year ago

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