CodeHawks Escrow Contract - Competition Details
Cyfrin
Foundry
40,000 USDC
View results
Previous Next
Submission Details
Severity: gas
Valid

Constants in comparisons should appear on the left side

theblockchainer

Summary

Constants on the left are better, but this is often trumped by a preference for English word order.

Vulnerability Details

Typically we all write comparison statements like this:

if (currentValue == 5)
{
// do work
}
But, the following is just as valid:
​
if (5 == currentValue)
{
// do work
}
The bottom style is a bit safer in languages like C because if you forget to put a double equals sign
if (currentValue = 5)
{
// do work
}

the compiler will assign 5 to the “currentValue” variable and the result will be the value of the assignment, which is 5.

Anything that isn’t zero is “truthy” and will cause the “if” branch to be taken. If you didn’t intend for this and you’re lucky enough to have compiler warnings turned all the way up, you’ll get a helpful message like “warning C4706: assignment within conditional expression.”

Instances: 7

  • Escrow.sol:

|if (address(tokenContract) == address(0)) revert Escrow__TokenZeroAddress();

|if (buyer == address(0)) revert Escrow__BuyerZeroAddress();

|if (seller == address(0)) revert Escrow__SellerZeroAddress();

|if (i_arbiter == address(0)) revert Escrow__DisputeRequiresArbiter();

if (buyerAward > 0) {
i_tokenContract.safeTransfer(i_buyer, buyerAward);
}
if (i_arbiterFee > 0) {
i_tokenContract.safeTransfer(i_arbiter, i_arbiterFee);
}
if (tokenBalance > 0) {
i_tokenContract.safeTransfer(i_seller, tokenBalance);
}

Impact

Placing the constants on the left will prevent typo bugs.

Tools Used

Manual finding

Recommendations

Change the order of the conditions such as:
if (address(0) == seller)

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.