The EscrowFactory contract is not transfering the arbiterFee to the escrow contract which might lead to some disputes not getting resolved.
Vulnerability Details
When we as a buyer call the EscrowFactory contract's newEscrow function then in the line (provided in the relevant Github section) then the function transfer tokens = price we want to pay to the seller instead we should be transfering the price + arbiter fee to the escrow contract.
Impact
There can be a case when the arbiter calls the initiatedispute function with the buyerAward = price (there can be a scenario when a buyer might receive the full price as a awards back for example he might not want the services of the seller maybe the seller is delaying the audit report and the buyer wants to get the funds deposited in the escrow contract and go to another auditor,only way for the buyer to get the funds is if the arbiter call the resolveDispute function with the buyer award equal to price(as there is no withdraw function),but as we only transfered funds equal to the price of the service we don't have enough funds in the contract to give the arbiter fees and thus the resolveDisputeFunction will revert on line 112 as totalFee would be buyeraward(equal to price+ arbiter fees) and it would certainly be higher than the tokenBalance in the contract.The most funds buyer can get = price - arbiterFee thus causing loss of funds for the buyer.
Tools Used
Manual review
Recommendations
EscrowFactory should transfer tokens = price + arbiter fees
If we do so we also need to correct our confirmReceipt function as it transfers tokenamount = balance of the contract which would give price + arbiter fees to the seller so we need to change the line i_tokenContract.safeTransfer(i_seller, i_tokenContract.balanceOf(address(this))) to i_tokenContract.safeTransfer(i_seller, price)
We then also need a way for the buyer to get the arbiterFees from the escrow contract if arbiter services were not required i.e there were no disputes so for that we can create a function = withdrawAfterconfirmreceipt() in which we transfer the arbiterFees left in the contract back to the buyer thus the function could only be called by the buyer and only when the s_state = State.confirmed so that only after we have paid the seller their price we can take the arbiterFees back otherwise it would be locked in the contract.
Not creating a new excrow contract with Enough funds
Severity
High Risk
Relevant GitHub Links
https://github.com/Cyfrin/2023-07-escrow/blob/main/src/EscrowFactory.sol#L39
https://github.com/Cyfrin/2023-07-escrow/blob/main/src/Escrow.sol#L112
https://github.com/Cyfrin/2023-07-escrow/blob/main/src/Escrow.sol#L98
Summary
The EscrowFactory contract is not transfering the arbiterFee to the escrow contract which might lead to some disputes not getting resolved.
Vulnerability Details
When we as a buyer call the EscrowFactory contract's newEscrow function then in the line (provided in the relevant Github section) then the function transfer tokens = price we want to pay to the seller instead we should be transfering the price + arbiter fee to the escrow contract.
Impact
There can be a case when the arbiter calls the initiatedispute function with the buyerAward = price (there can be a scenario when a buyer might receive the full price as a awards back for example he might not want the services of the seller maybe the seller is delaying the audit report and the buyer wants to get the funds deposited in the escrow contract and go to another auditor,only way for the buyer to get the funds is if the arbiter call the resolveDispute function with the buyer award equal to price(as there is no withdraw function),but as we only transfered funds equal to the price of the service we don't have enough funds in the contract to give the arbiter fees and thus the resolveDisputeFunction will revert on line 112 as totalFee would be buyeraward(equal to price+ arbiter fees) and it would certainly be higher than the tokenBalance in the contract.The most funds buyer can get = price - arbiterFee thus causing loss of funds for the buyer.
Tools Used
Manual review
Recommendations
EscrowFactory should transfer tokens = price + arbiter fees
If we do so we also need to correct our confirmReceipt function as it transfers tokenamount = balance of the contract which would give price + arbiter fees to the seller so we need to change the line i_tokenContract.safeTransfer(i_seller, i_tokenContract.balanceOf(address(this))) to i_tokenContract.safeTransfer(i_seller, price) We then also need a way for the buyer to get the arbiterFees from the escrow contract if arbiter services were not required i.e there were no disputes so for that we can create a function = withdrawAfterconfirmreceipt() in which we transfer the arbiterFees left in the contract back to the buyer thus the function could only be called by the buyer and only when the s_state = State.confirmed so that only after we have paid the seller their price we can take the arbiterFees back otherwise it would be locked in the contract.