Submission Details
Severity:
No way to safely cancel agreement if needed.
Summary
If for some reason the two parties agree to cancel the agreement, eg. Illness, time constraints, family responsibility.
There is no way to cancel the agreement and refund the funds to the BUYER, without either raising a dispute and loosing fees or needing to trust the SELLER to refund the funds to the BUYER after confirmReceipt is called.
Vulnerability Details
Impact
The BUYER would either loose the fees portion of their funds, or need to trust the SELLER to refund their funds.
Tools Used
Manual Audit
Recommendations
You could consider adding a cancellation function that requires mutual approval.
CODE and Foundry test below example of function
function cancelAgreement() external onlyBuyerOrSeller inState(State.Created) {
if(msg.sender == i_seller)
{
sellerCancelled = true;
}
if(msg.sender == i_buyer)
{
buyerCancelled = true;
}
if(buyerCancelled && sellerCancelled)
{
s_state = State.Cancelled;
uint256 contractBalance = i_tokenContract.balanceOf(address(this));
if (contractBalance > 0) {
i_tokenContract.safeTransfer(i_buyer, contractBalance);
}
}
emit Cancelled(i_buyer, i_seller);
}
IEscrow contract
// SPDX-License-Identifier: MIT
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
interface IEscrow {
/// Errors
error Escrow__FeeExceedsPrice(uint256 price, uint256 fee);
error Escrow__OnlyBuyer();
error Escrow__OnlyBuyerOrSeller();
error Escrow__OnlyArbiter();
error Escrow__InWrongState(State currentState, State expectedState);
error Escrow__MustDeployWithTokenBalance();
error Escrow__TotalFeeExceedsBalance(uint256 balance, uint256 totalFee);
error Escrow__DisputeRequiresArbiter();
error Escrow__TokenZeroAddress();
error Escrow__BuyerZeroAddress();
error Escrow__SellerZeroAddress();
event Confirmed(address indexed seller);
event Disputed(address indexed disputer);
event Resolved(address indexed buyer, address indexed seller);
event Cancelled(address indexed buyer, address indexed seller);
enum State {
Created,
Confirmed,
Disputed,
Resolved,
Cancelled
}
/// @dev Buyer confirms receipt from seller; token `price` is transferred to seller.
function confirmReceipt() external;
/// @dev Buyer or seller can initiate dispute related to transactions,
/// placing `price` transfer and split of value into arbiter control.
/// For example, buyer might refuse or unduly delay to confirm receipt after seller delivery,
/// or, on other hand, despite buyer's dissatisfaction with seller delivery,
/// seller might demand buyer confirm receipt and release `price`.
function initiateDispute() external;
/// @dev Buyer and seller must confirm cancellation,
/// Cancelling will trigger a refund to the BUYER.
function cancelAgreement() external;
/// @notice Arbiter can resolve dispute and claim token reward by entering in split of `price` value,
/// minus `arbiterFee` set at construction.
function resolveDispute(uint256 buyerAward) external;
/////////////////////
// View functions
/////////////////////
function getPrice() external view returns (uint256);
function getTokenContract() external view returns (IERC20);
function getBuyer() external view returns (address);
function getSeller() external view returns (address);
function getArbiter() external view returns (address);
function getArbiterFee() external view returns (uint256);
function getState() external view returns (State);
}
Escrow contract
// SPDX-License-Identifier: MIT
// Inspired by `BillOfSaleERC20` contract: https://github.com/open-esq/Digital-Organization-Designs/blob/master/Finance/BillofSaleERC20.sol
import {IEscrow} from "./IEscrow.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
/// @author Cyfrin
/// @title Escrow
/// @notice Escrow contract for transactions between a seller, buyer, and optional arbiter.
contract Escrow is IEscrow, ReentrancyGuard {
using SafeERC20 for IERC20;
uint256 private immutable i_price;
/// @dev There is a risk that if a malicious token is used, the dispute process could be manipulated.
/// Therefore, careful consideration should be taken when chosing the token.
IERC20 private immutable i_tokenContract;
address private immutable i_buyer;
address private immutable i_seller;
address private immutable i_arbiter;
uint256 private immutable i_arbiterFee;
bool private buyerCancelled;
bool private sellerCancelled;
State private s_state;
/// @dev Sets the Escrow transaction values for `price`, `tokenContract`, `buyer`, `seller`, `arbiter`, `arbiterFee`. All of
/// these values are immutable: they can only be set once during construction and reflect essential deal terms.
/// @dev Funds should be sent to this address prior to its deployment, via create2. The constructor checks that the tokens have
/// been sent to this address.
constructor(
uint256 price,
IERC20 tokenContract,
address buyer,
address seller,
address arbiter,
uint256 arbiterFee
) {
if (address(tokenContract) == address(0)) revert Escrow__TokenZeroAddress();
if (buyer == address(0)) revert Escrow__BuyerZeroAddress();
if (seller == address(0)) revert Escrow__SellerZeroAddress();
if (arbiterFee >= price) revert Escrow__FeeExceedsPrice(price, arbiterFee);
if (tokenContract.balanceOf(address(this)) < price) revert Escrow__MustDeployWithTokenBalance();
i_price = price;
i_tokenContract = tokenContract;
i_buyer = buyer;
i_seller = seller;
i_arbiter = arbiter;
i_arbiterFee = arbiterFee;
}
/////////////////////
// Modifiers
/////////////////////
/// @dev Throws if called by any account other than buyer.
modifier onlyBuyer() {
if (msg.sender != i_buyer) {
revert Escrow__OnlyBuyer();
}
_;
}
/// @dev Throws if called by any account other than buyer or seller.
modifier onlyBuyerOrSeller() {
if (msg.sender != i_buyer && msg.sender != i_seller) {
revert Escrow__OnlyBuyerOrSeller();
}
_;
}
/// @dev Throws if called by any account other than arbiter.
modifier onlyArbiter() {
if (msg.sender != i_arbiter) {
revert Escrow__OnlyArbiter();
}
_;
}
/// @dev Throws if contract called in State other than one associated for function.
modifier inState(State expectedState) {
if (s_state != expectedState) {
revert Escrow__InWrongState(s_state, expectedState);
}
_;
}
/////////////////////
// Functions
/////////////////////
/// @inheritdoc IEscrow
function confirmReceipt() external onlyBuyer inState(State.Created) {
s_state = State.Confirmed;
emit Confirmed(i_seller);
i_tokenContract.safeTransfer(i_seller, i_tokenContract.balanceOf(address(this)));
}
/// @inheritdoc IEscrow
function initiateDispute() external onlyBuyerOrSeller inState(State.Created) {
if (i_arbiter == address(0)) revert Escrow__DisputeRequiresArbiter();
s_state = State.Disputed;
emit Disputed(msg.sender);
}
/// @inheritdoc IEscrow
function cancelAgreement() external onlyBuyerOrSeller inState(State.Created) {
if(msg.sender == i_seller)
{
sellerCancelled = true;
}
if(msg.sender == i_buyer)
{
buyerCancelled = true;
}
if(buyerCancelled && sellerCancelled)
{
s_state = State.Cancelled;
uint256 contractBalance = i_tokenContract.balanceOf(address(this));
if (contractBalance > 0) {
i_tokenContract.safeTransfer(i_buyer, contractBalance);
}
}
emit Cancelled(i_buyer, i_seller);
}
/// @inheritdoc IEscrow
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);
}
s_state = State.Resolved;
emit Resolved(i_buyer, i_seller);
if (buyerAward > 0) {
i_tokenContract.safeTransfer(i_buyer, buyerAward);
}
if (i_arbiterFee > 0) {
i_tokenContract.safeTransfer(i_arbiter, i_arbiterFee);
}
tokenBalance = i_tokenContract.balanceOf(address(this));
if (tokenBalance > 0) {
i_tokenContract.safeTransfer(i_seller, tokenBalance);
}
}
/////////////////////
// View functions
/////////////////////
function getPrice() external view returns (uint256) {
return i_price;
}
function getTokenContract() external view returns (IERC20) {
return i_tokenContract;
}
function getBuyer() external view returns (address) {
return i_buyer;
}
function getSeller() external view returns (address) {
return i_seller;
}
function getArbiter() external view returns (address) {
return i_arbiter;
}
function getArbiterFee() external view returns (uint256) {
return i_arbiterFee;
}
function getState() external view returns (State) {
return s_state;
}
}
Foundry test
token contract below this contract
(copy/past to file EscrowTokenTest.sol)
// SPDX-License-Identifier: MIT
import "forge-std/Test.sol";
import {IEscrow,Escrow} from "../src/Escrow.sol";
import {EscrowFactory} from "../src/EscrowFactory.sol";
import "../src/TestToken.sol";
contract EscrowTokenTest is Test {
bytes32 public constant SALT1 = bytes32(uint256(keccak256(abi.encodePacked("test"))));
EscrowFactory public escrowFactory;
address public DEPLOYER = makeAddr("DEPLOYER");
address public BUYER = makeAddr("BUYER");
address public SELLER= makeAddr("SELLER");
address public ARBITER = makeAddr("ARBITER");
uint256 public constant ARBITER_FEE = 1e16;
IEscrow public escrow;
uint256 public buyerAward = 0;
TestToken testToken;
// events
event Confirmed(address indexed seller);
event Disputed(address indexed disputer);
event Resolved(address indexed buyer, address indexed seller);
event Cancelled(address indexed buyer, address indexed seller);
function setUp() external {
vm.startPrank(DEPLOYER);
//Create ERC20 token
testToken = new TestToken();
// Mint tokens to parties
testToken.mint(BUYER,100 ether);
testToken.mint(SELLER,10 ether);
escrowFactory = new EscrowFactory();
vm.stopPrank();
}
function testHappyPath() public {
vm.startPrank(BUYER);
testToken.approve(address(escrowFactory), 10 ether);
escrow = escrowFactory.newEscrow(10 ether, IERC20(testToken), SELLER, ARBITER, ARBITER_FEE, SALT1);
vm.stopPrank();
vm.startPrank(BUYER);
uint256 balanceBefore = testToken.balanceOf(SELLER);
escrow.confirmReceipt();
uint256 balanceAfter = testToken.balanceOf(SELLER);
vm.stopPrank();
assertGt(balanceAfter,balanceBefore);
assertEq(escrow.getPrice(), 10 ether);
assertEq(address(escrow.getTokenContract()), address(testToken));
assertEq(escrow.getBuyer(), BUYER);
assertEq(escrow.getSeller(), SELLER);
assertEq(escrow.getArbiter(), ARBITER);
assertEq(escrow.getArbiterFee(), ARBITER_FEE);
}
function testCancelledPath() public {
vm.startPrank(BUYER);
testToken.approve(address(escrowFactory), 10 ether);
escrow = escrowFactory.newEscrow(10 ether, IERC20(testToken), SELLER, ARBITER, ARBITER_FEE, SALT1);
vm.stopPrank();
//first get balance of BUYER to confirm refund
uint256 balanceBefore = testToken.balanceOf(BUYER);
vm.startPrank(BUYER);
escrow.cancelAgreement();
vm.stopPrank();
vm.startPrank(SELLER);
// check the cancelled event is emitted
vm.expectEmit(address(escrow));
emit Cancelled(BUYER,SELLER);
// this will be the 2nd confirmation so the refund and event should happen here
escrow.cancelAgreement();
vm.stopPrank();
// now get BUYER balance to check refund has occurred
uint256 balanceAfter = testToken.balanceOf(BUYER);
assertGt(balanceAfter,balanceBefore);
}
}
TestToken contract (copy/past to file TestToken.sol)
// SPDX-License-Identifier: UNLICENSED
// slither-disable-next-line solc-version
import "../lib/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
/// @dev Lottery token contract. The token has a fixed initial supply.
/// Additional tokens can be minted after each draw is finalized. Inflation rates (per draw) are defined for each year.
contract TestToken is ERC20 {
uint256 public constant INITIAL_SUPPLY = 1_000_000_000e18;
address _owner;
constructor() ERC20("TToken", "TTT") {
_owner = msg.sender;
}
//modifier
modifier onlyOwner() {
require(msg.sender==_owner,"Notthe Owner!");
_;
}
function mint(address account, uint256 amount) external onlyOwner {
_mint(account, amount);
}
}
Prize pool breakdown
Total prize
40,000 USDC
nSLOC
186215 USDC / LOC
37,000 USDC
3,000 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.