code-423n4 / 2023-10-wildcat-findings

12 stars 9 forks source link

Based on the functionality, if the `releaseEscrow()` function can be called by unauthorized entities, it can lead to potential misuse or unintended transfer of assets. #707

Closed c4-submissions closed 10 months ago

c4-submissions commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/main/src/WildcatSanctionsEscrow.sol#L25

Vulnerability details

Impact

The absence of access control on the releaseEscrow() function presents a significant security risk. As it currently stands, any external actor or contract can invoke this function, which may result in the unintended release of escrowed funds. This opens up potential attack vectors where malicious actors can drain the funds stored in the contract without meeting the intended conditions for fund release.

Given that escrows are typically used to hold significant amounts of assets in a trustless environment (pending certain conditions are met), the exploitation of this vulnerability can lead to substantial financial losses for the stakeholders involved. Additionally, it can undermine trust in the platform or system that utilizes this contract, deterring potential users and stakeholders from engaging with it in the future.

Proof of Concept

Deployment of Vulnerable Contract: Deploy the WildcatSanctionsEscrow contract on a test network.

Populate the Escrow: Send a significant amount of assets to the contract address to simulate an escrowed amount.

Unauthorized Release: Using an unrelated Ethereum address (one that is neither the borrower nor the account), invoke the releaseEscrow() function.

Verify Asset Transfer: Check the balance of the account address. If the assets have been transferred without the intended conditions being met, the vulnerability is confirmed.

GitHub Link: WildcatSanctionsEscrow.sol

 function canReleaseEscrow() public view override returns (bool) {
    return !WildcatSanctionsSentinel(sentinel).isSanctioned(borrower, account);
  }

  function escrowedAsset() public view override returns (address, uint256) {
    return (asset, balance());
  }

  function releaseEscrow() public override {
    if (!canReleaseEscrow()) revert CanNotReleaseEscrow();

    uint256 amount = balance();

    IERC20(asset).transfer(account, amount);

    emit EscrowReleased(account, asset, amount);
  }

Tools Used

VS code

Recommended Mitigation Steps

Implement Access Control:

Introduce a modifier in the WildcatSanctionsEscrow contract that restricts access to the releaseEscrow() function. This modifier should only allow specific addresses (e.g., the borrower or the account) to call the function.

modifier onlyAuthorized() {
    require(msg.sender == borrower || msg.sender == account, "Not authorized");
    _;
}

Update the Vulnerable Function:

Apply the newly created onlyAuthorized modifier to the releaseEscrow() function:

function releaseEscrow() public onlyAuthorized override {
    ...
}

Testing:

After making the changes, it's crucial to test the contract thoroughly. Ensure that only the allowed addresses can call releaseEscrow() and that unauthorized addresses are denied. Employ unit tests and, if possible, automated testing frameworks like Truffle or Hardhat to ensure that the changes work as expected.

Assessed type

Invalid Validation

c4-pre-sort commented 10 months ago

minhquanym marked the issue as low quality report

minhquanym commented 10 months ago

AI generated. Invalid

laurenceday commented 10 months ago

The year is 2058.

I have retired, and am sitting at home surrounded by my adult children, my loving wife and my pets. I have not actively worked on the Wildcat protocol for over thirty years now.

The envelope in the top left corner of my vision vibrates, a sign that I have received a message directly to my Apple iPatch. I twitch my eye and the message trickles up like the introductory text to a Star Wars film ('films', I tell my children, 'were like TikToks, but they were sometimes over 90 minutes long').

It is the very last gratuitous finding from the protocol audit that I commissioned long ago, a reminder finally having reached my inbox after the message finally bounced into the proximity of a Starlink satellite in Earth's orbit (the Starlink network, I dig through dusty corners of my mind to remember, was scattered across the Solar System in the Great Proxy War of 2031).

"ur releaseEscrow() function can be activated by anyone and someone can steal all ur money (HIGH RISK)"

I sigh, and - with my eyes picking out letters from a keyboard visible only to me - belt out a response, a terse, written-a-thousand-times-before "please read the if-statement condition - not a bug, WONTFIX".

I tilt my head backwards, and a faint whooshing sound in my ear tells me that the response is on its way. Hopefully it can find a way to join the information superhighway via a less time-consuming manner on the way back: perhaps via the Eliezer Yudkowsky Quantum AI Optic Network. I can hope, anyway.

I turn back to my family, and I scratch the ears of my cat, who has settled onto my lap in the thirty seconds it took to do this.

My task is finished. I can finally rest.

c4-judge commented 10 months ago

MarioPoneder marked the issue as unsatisfactory: Invalid