Inverse Finance contest - minhtrng's results

Rethink the way you borrow.

General Information

Platform: Code4rena

Start Date: 25/10/2022

Pot Size: $50,000 USDC

Total HM: 18

Participants: 127

Period: 5 days

Judge: 0xean

Total Solo HM: 9

Id: 175

League: ETH

Inverse Finance

Findings Distribution

Researcher Performance

Rank: 29/127

Findings: 2

Award: $343.35

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

🌟 Selected for report: neumo

Also found by: BClabs, ladboy233, minhtrng, rvierdiiev

Labels

bug
2 (Med Risk)
satisfactory
duplicate-379

Awards

342.9734 USDC - $342.97

External Links

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/escrows/GovTokenEscrow.sol#L19 https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/escrows/SimpleERC20Escrow.sol#L16

Vulnerability details

Impact

The market currently assumes that all escrow implementations comply with the IEscrow interface. This is currently not the case, which can lead to the deposit functionality being bricked.

Proof of Concept

If the callOnDepositCallback function is set to true during construction of a market, the deposit function will attempt to call onDeposit on the escrow:

function deposit(address user, uint amount) public {
    IEscrow escrow = getEscrow(user);
    collateral.transferFrom(msg.sender, address(escrow), amount);
    if(callOnDepositCallback) {
        escrow.onDeposit();
    }
    emit Deposit(user, amount);
}

Currently, only INVEscrow implements that function, while SimpleERC20Escrow and GovTokenEscrow do not. If they were to be used on a market with said callOnDepositCallback set to true the deposit function would always revert.

Tools Used

Manual Review

Ensure that all escrow contracts inherit from IEscrow to have compile time safety and get immediate feedback on a contract being non-compliant. For the contracts that do not need a certain function, such as onDeploy in this case, leave the function body empty.

#0 - c4-judge

2022-11-05T22:44:27Z

0xean marked the issue as duplicate

#1 - Simon-Busch

2022-12-05T15:31:29Z

Issue marked as satisfactory as requested by 0xean

#2 - c4-judge

2022-12-07T08:23:39Z

Simon-Busch marked the issue as duplicate of #379

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Oracle.sol#L6 https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Oracle.sol#L82 https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Oracle.sol#L116

Vulnerability details

Impact

The chainlink data is currently requested through an deprecated endpoint. Also, there are no checks to the validity of the data. Stale data could lead to users borrowing more than they should be allowed to, or users being liquidated even though they have enough collateral.

Proof of Concept

The Oracle currently uses the latestAnswer function of a Chainlink feed to retrieve current prices:

function viewPrice(address token, uint collateralFactorBps) external view returns (uint) {
    ...
    uint price = feeds[token].feed.latestAnswer();
    ...
}

function getPrice(address token, uint collateralFactorBps) external returns (uint) {
    ...
    uint price = feeds[token].feed.latestAnswer();
    ...
}

This function is deprecated according to Chainlinks API reference for price feeds and offers no information on the latest time of update.

Tools Used

Manual Review

Use latestRoundData instead (refer to Chainlink docs linked above). This function also returns the time of update, which can be used to check for staleness of data, which should be handled (for example by emitting events or having a pause mechanism for extended staleness).

#0 - neumoxx

2022-10-31T08:39:00Z

Duplicate of #601

#1 - c4-judge

2022-11-05T17:48:47Z

0xean marked the issue as duplicate

#2 - Simon-Busch

2022-12-05T15:29:31Z

Issue marked as satisfactory as requested by 0xean

#3 - c4-judge

2022-12-07T08:14:13Z

Simon-Busch marked the issue as duplicate of #584

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter