Inverse Finance contest - c7e7eff's results

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L136

Vulnerability details

Impact

Currently governance roles are used in multiple high risk parameter changes like changing the collateral and liquidation factors. All these changes go into effect immediatly. In case of a key compromise this could be used to set the lender and removing all of the DOLA funds held by the Market, but also set the liquidation factor etc.

Proof of Concept

In the case of the Market contract the Governance can set the the LiquidationFactor, set the lender who in turn can transfer a large amount of DOLA from the market to themselves)

setLender() https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L136 recal() https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L203

setLiquidatoinFactor https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L161

setReplenishmentIncentive() https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L172 setLiquidationIncentiveBps() https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L183

In the case of the DBR contract the Operator can set the minter which in turn can mint any amount of DBR tokens to any address addMinter() https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/DBR.sol#L81 mint() https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/DBR.sol#L349

In the case of the oracle contract the operator can set a fixed price or change the feed for any token of the oracle: setFixedPrice() https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Oracle.sol#L61 setFeed(): https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Oracle.sol#L53

Tools Used

Manual review.

Recommended Mitigation Steps

To protect against the impact of a possible governance key compromise placing a timelock on certain functions allows for proper action by the team and allow users to repay their loans and withdraw their collateral.

Lines of code

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

Vulnerability details

Impact

Chainlink has deprecated their latestAnswer() API in favor of the latestRoundData(). The returned data should also be checked to be recent. Stale data could lead to a miscalculation of collateral value.

Proof of Concept

The Oracle's GetPrice() uses a deprecated function of the Chainlink API

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

Tools Used

Manual review

Recommended Mitigation Steps

Use the latestRoundData() and check if the data is recent by comparing the roundIdand answeredInRound values.

function getPrice(address token, uint collateralFactorBps) external returns (uint) {
	...
	uint price = feeds[token].feed.latestAnswer();
	require(answeredInRound >= roundID, "Stale price data.");

Inverse Finance QA report

Low

L00 Use timestamp instead of nonce for EIP712 signed structures

• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L438

EIP is meant to be make the message to sign human readable. A nonce doesn't mean anything to a human. Using a signedAt field in the form of a timestamp (similar to the deadline) makes it possible for wallets to present this information as human readable date and time. The increasing the nonce can then be replaced by setting an 'invalid before' timestamp before which all signed messages become invalid.

L01 Change of governance/operator should be a 2 step approach

• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/BorrowController.sol#L26
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Fed.sol#L48
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L130

Setting the governance or operator address in 1 step is prone to setting it to the wrong address in which case it cannot be recovered. A 2 step approach (setPending/claim) is recommended as is already done in some other contracts. Personally I feel this qualifies at least as a medium risk, but I know there is some discussion about the risk level so I have put is as a low risk.

L02 Feed tokenDecimals should be taken form the token contract itself

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

The decimals for the collateral ERC20 token should be taken from the contract itself instead of taking it as a parameter. The token contract is given as the first parameter so calling decimals() on the token contract eliminates any possible misconfiguration.

L03 Governance and operator functions do not emit events

• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/BorrowController.sol#L26
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/BorrowController.sol#L32
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/BorrowController.sol#L38
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/DBR.sol#L53
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/DBR.sol#L62
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Fed.sol#L57
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Fed.sol#L66
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Fed.sol#L75
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L118
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L124
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L130
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L136
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L142
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L149
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L161
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L172
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L183
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L194
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L212
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Oracle.sol#L44
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Oracle.sol#L53
• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Oracle.sol#L61

Multiple governance functions do not emit appropriate events. These events allow monitoring of certain actions taken on the contract.

Non Critical

NC00

• https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Oracle.sol#L108-L112

no @param specified for collateralFactorBps