Holograph contest - d3e4's results

Lines of code

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

Vulnerability details

Impact

Payout is blocked.

Proof of Concept

PA1D._payoutToken() and PA1D._payoutTokens() call ERC20.transfer() to send tokens to a list of payout recipients. Some tokens (e.g. LEND) revert when transferring a zero value amount. If one of the recipients is to receive a zero amount of such a token, then the entire transaction will revert and the payout cannot be made.

Tools Used

Code inspection

Recommended Mitigation Steps

Only attempt to transfer to a recipient if the amount (sending) is greater than zero.

Lines of code

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

Vulnerability details

Impact

Payout is blocked and tokens are stuck in contract.

Proof of Concept

PA1D._payoutToken() and PA1D._payoutTokens() call ERC20.transfer() in a require-statement to send tokens to a list of payout recipients. Some tokens do not return a bool (e.g. USDT, BNB, OMG) on ERC20 methods. But since the require-statement expects a bool, for such a token a void return will also cause a revert, despite an otherwise successful transfer. That is, the token payout will always revert for such tokens.

Tools Used

Code inspection

Recommended Mitigation Steps

Use OpenZeppelin's SafeERC20, which handles the return value check as well as non-standard-compliant tokens.

Lines of code

https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographOperator.sol#L400

Vulnerability details

Impact

The miner can chose pod with high probability.

Proof of Concept

The random number generation can be easily influenced by the miner by setting block.timestamp to within an acceptable value ranging some 10-15 consecutive values (seconds). Since this is hashed into a "random" number he will likely have some 10 different final random to chose from (different up to modulo the number of pods). Since this number is reduced modulo the number of pods it will give him a guaranteed way to chose the pod, by setting the timestamp, if the number of pods is up to around 10, and a significant chance of selecting his desired pod if they are more.

      uint256 random = uint256(keccak256(abi.encodePacked(jobHash, _jobNonce(), block.number, block.timestamp)));

Tools Used

Code inspection

Recommended Mitigation Steps

Use a safe way of getting random numbers (such as ChainLink VRF), or in this case it might be sufficient to simply remove the block.timestamp. The miner can still influence the result by deciding not to publish a block but this is probably not profitably for him.

Lines of code

https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L291 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L312 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L336

Vulnerability details

Impact

Payouts from PA1D.sol cannot be made unless the balance is sufficiently high, meaning

Proof of Concept

In PA1D.sol the payout functions _payoutEth(), _payoutToken() and _payoutTokens() require that balance > 10000. If this is not met the recipients cannot receive their payment. This might be negligible in the case of ETH, but not necessarily for ERC20-token payouts. For example Gemini USD has only two decimals, so a value of up to 100 USD would be stuck in the contract.

Similarly in PA1D._payoutTokens().

Tools Used

Code inspection

Recommended Mitigation Steps

This require-check should not be necessary. Rounding errors occur regardless and there is no specific reason payout shouldn't be possible for a smaller amount.

[L-01] Loss of precision from dividing before multiplying

Change HolographOperator.sol#L1078: current += (current / _operatorThresholdDivisor) * (position / _operatorThresholdStep); into current += current * position / _operatorThresholdStep / _operatorThresholdDivisor;.

[L-02] Function returns (bool) but cannot return false

The following functions returns (bool) but either return true or reverts. They cannot return false. This may cause problems if they are elsewhere expected to return false rather than revert.

HolographERC20.approve() HolographERC20.burnFrom() HolographERC20.decreaseAllowance() HolographERC20.increaseAllowance() HolographERC20.safeTransfer() HolographERC20.safeTransfer() HolographERC20.safeTransferFrom() HolographERC20.safeTransferFrom() HolographERC20.transfer() HolographERC20.transferFrom()

[N-01] Constants should be defined rather than using magic numbers

Instances: https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographOperator.sol#L155 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographOperator.sol#L259 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L312 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L316

[N-02] Constants should be in UPPER_CASE_WITH_UNDERSCORES

Instances:

https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/abstract/ERC20H.sol#L11 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/abstract/ERC20H.sol#L15 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/abstract/ERC721H.sol#L11 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/abstract/ERC721H.sol#L15 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/Holographer.sol#L20 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/Holographer.sol#L24 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/Holographer.sol#L28 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/Holographer.sol#L32 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/Holographer.sol#L36 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/HolographERC20.sol#L43 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/HolographERC20.sol#L47 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/HolographERC721.sol#L37 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/HolographERC721.sol#L41 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L25 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L29 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L33 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L37 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L41 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L43 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L44 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L45 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographBridge.sol#L27 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographBridge.sol#L31 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographBridge.sol#L35 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographBridge.sol#L39 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographBridge.sol#L43 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographFactory.sol#L28 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographFactory.sol#L32 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographOperator.sol#L30 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographOperator.sol#L34 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographOperator.sol#L38 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographOperator.sol#L42 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographOperator.sol#L46 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographOperator.sol#L50 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographOperator.sol#L54 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/module/LayerZeroModule.sol#L27 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/module/LayerZeroModule.sol#L31 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/module/LayerZeroModule.sol#L35 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/module/LayerZeroModule.sol#L39 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/module/LayerZeroModule.sol#L43 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/module/LayerZeroModule.sol#L47

[N-03] Typos

initilaization -> initialization addresse's -> addresses' initilaization -> initialization domainSeperator -> domainSeparator domainSeperator -> domainSeparator initilaization -> initialization coud -> could missmatch -> mismatch transfered -> transferred usecases -> use cases accesible -> accessible initilaization -> initialization optimizaion -> optimization accomodate -> accommodate tranaction -> transaction missmatched -> mismatched lenghts -> lengths equal or greater than -> equal to or greater than initilaization -> initialization brigeOutRequest -> bridgeOutRequest destiantion -> destination destiantion -> destination transfered -> transferred initilaization -> initialization deployement -> deployment transfered -> transferred initilaization -> initialization cross chain -> cross-chain accomodate -> accommodate rever -> revert cross chain -> cross-chain destiantion -> destination destiantion -> destination adress -> address initilaization -> initialization transfered -> transferred

[N-04] British spellings inconsistent with otherwise American spellings

utilised -> utilized authorisations -> authorizations authorised -> authorized

[G-01] Cache variable in memory

msg.sender in PA1D.sol#L107-L110

_operatorTempStorageCounter at L396, L418 and L425 in HolographOperator.crossChainMessage() and store new value afterwards.

[G-02] Don't calculate balance - gasCost twice

Change https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L291-L292:

require(balance - gasCost > 10000, "PA1D: Not enough ETH to transfer");
balance = balance - gasCost;

into

balance = balance - gasCost;
require(balance > 10000, "PA1D: Not enough ETH to transfer");

[G-03] Unused code

Instances: PA1D._setTokenAddress() HolographOperator._interfaces() LayerZeroModule._bridge()

[G-04] Declaring constants as private rather than public saves deployment gas

The compiler will give public constants a getter method, which costs gas during deployment. As they are constants they can be read directly from the verified source code.

Instances: https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/abstract/ERC20H.sol#L11 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/abstract/ERC20H.sol#L15 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/abstract/ERC721H.sol#L11 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/abstract/ERC721H.sol#L15 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/Holographer.sol#L20 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/Holographer.sol#L24 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/Holographer.sol#L28 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/Holographer.sol#L32 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/Holographer.sol#L36 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/HolographERC20.sol#L43 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/HolographERC20.sol#L47 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/HolographERC721.sol#L37 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/HolographERC721.sol#L41 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L25 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L29 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L33 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L37 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L41 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L43 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L44 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/enforcer/PA1D.sol#L45 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographBridge.sol#L27 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographBridge.sol#L31 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographBridge.sol#L35 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographBridge.sol#L39 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographBridge.sol#L43 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographFactory.sol#L28 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographFactory.sol#L32 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographOperator.sol#L30 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographOperator.sol#L34 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographOperator.sol#L38 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographOperator.sol#L42 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographOperator.sol#L46 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographOperator.sol#L50 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographOperator.sol#L54 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/module/LayerZeroModule.sol#L27 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/module/LayerZeroModule.sol#L31 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/module/LayerZeroModule.sol#L35 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/module/LayerZeroModule.sol#L39 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/module/LayerZeroModule.sol#L43 https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/module/LayerZeroModule.sol#L47