Unchecked return values in setValueForNextCall

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/ContractDeployer.sol#L320

Vulnerability details

Title:

Description: When creating a contract, if there is value to be transferred the _constructContract function of ContractDeployer will use the SystemContractsHelper.setValueForNextFarCall Method: https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/libraries/SystemContractHelper.sol#L161-L172

However in _constructContract function doesn't verify that the value returned was success: https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/ContractDeployer.sol#L320

As a result this could lead to an incorrect handling of msg.value and balances for contracts. Currently this may not lead to loss of funds since the L2EthToken contract is called and the balance is successfully transferred there, but this could have downstream consequences if the created contract uses the msg.value variable in some way.

Tools Used

Manual Review

Recommended Mitigation Steps

Check the return value and revert the transaction if success != true.

[GalloDaSballo]

I believe the lack of checks can open up to OOG griefs

[c4-sponsor]

miladpiri marked the issue as sponsor disputed

[miladpiri]

It is a compiler simulation and if it fails, the frame will fail too. Moreover, The compiler simulation doesn’t return the meaningful value. It is different from a normal low level call.

[GalloDaSballo]

From the Sponsor Provided Docs, we can verify that the call will transpile to:

Am closing as invalid due to the lack of acknowledgment of the zkCompiler Behaviour

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid