Summary

The computeEscrowAddress view in the contract, intended to predict the address of a new Escrow contract for off-chain use, is error-prone due to its dependency on compiler version and settings. It can cause unintended loss of funds if used for pre-funding the contracts.

Vulnerability Details

The problem arises due to the dependency on the Solidity compiler used to generate this bytecode. Different compiler versions or different compiler settings (like optimizations) can produce different bytecodes for the same Solidity source code. Therefore, when called from outside the contract, e.g., off-chain, if the bytecode is generated using a compiler version or settings different from the ones used when the contract was deployed, the computed address will not match the actual deployed contract address. Consequently, any funds sent to the incorrectly computed address would be irretrievably lost, as there would be no way to access or recover them.

Severity

High impact and low likelihood result in a medium severity.

Impact

This vulnerability has a high impact, as it can lead to irreversible loss of funds. If a user attempts to pre-fund an Escrow contract using the address generated by the computeEscrowAddress function and this address turns out to be incorrect, those funds become inaccessible and are effectively lost.

Likelihood

The likelihood of occurrence is low due to the expected reliance on pulled funds in newEscrow. However, users who do not rely on newEscrow to pull the full amount of funds may suffer loss of funds. Some reasons to avoid the default: reducing gas costs needed for the double transfer (and the approval call), taking advantage of bulk and batched transfers, avoiding granting token approvals.

Tools Used

Manual analysis.

Recommendations

Consider to modify computeEscrowAddress function to not require the bytecode as an input. Instead, it should use the bytecode of the Escrow contract known to it through type(Escrow).creationCode.