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

As some tokens balances can be changed over time, arbiter fee must be a percent of the price instead of the fixed value

mahdirostami

Summary

As i_price and actual balance can be different because of some Tokens which modify balances outside transfer operations like UBI, AMPL, or aTokens from AAVE, arbiterfee must be a percent of the price.

Vulnerability Details

In constructor, it checks that

if (arbiterFee >= price) revert Escrow__FeeExceedsPrice(price, arbiterFee);

But some tokens balances can be changed over time, so if the contract is in Disputed state and if the arbiter fee is more than the actual price, because of the:

function resolveDispute(uint256 buyerAward) external onlyArbiter nonReentrant inState(State.Disputed) {
uint256 tokenBalance = i_tokenContract.balanceOf(address(this));
uint256 totalFee = buyerAward + i_arbiterFee; // Reverts on overflow
if (totalFee > tokenBalance) {
revert Escrow__TotalFeeExceedsBalance(tokenBalance, totalFee);
}

resolveDispute function will revert, and All funds will lock in a contract. So the arbiter fee could be a percent of the price instead of a fixed value.

Impact

All funds will be locked in a contract.
OR

If a token balance will be changed, it must change arbiterFee as well.

Tools Used

Manual review

Recommendations

- if (arbiterFee >= price) revert Escrow__FeeExceedsPrice(price, arbiterFee);
+ if (arbiterFee >= 100) revert();
function resolveDispute(uint256 buyerAward) external onlyArbiter nonReentrant inState(State.Disputed) {
uint256 tokenBalance = i_tokenContract.balanceOf(address(this));
- uint256 totalFee = buyerAward + i_arbiterFee; // Reverts on overflow
+ uint256 arbiterFee = (i_arbiterFee * tokenBalance) / 100;
+ uint256 totalFee = buyerAward + arbiterFee; // Reverts on overflow
if (totalFee > tokenBalance) {
revert Escrow__TotalFeeExceedsBalance(tokenBalance, totalFee);
}
@@ -119,8 +120,8 @@ contract Escrow is IEscrow, ReentrancyGuard {
if (buyerAward > 0) {
i_tokenContract.safeTransfer(i_buyer, buyerAward);
}
- if (i_arbiterFee > 0) {
- i_tokenContract.safeTransfer(i_arbiter, i_arbiterFee);
+ if (arbiterFee > 0) {
+ i_tokenContract.safeTransfer(i_arbiter, arbiterFee);
}

Support

FAQs

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