code-423n4 / 2022-10-inverse-findings

0 stars 0 forks source link

QA Report #295

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Non-critical

Contract is missing events for certain actions

Locations 1 2 3 4 5 6 7 8 9 10 11 12 13 14

Description

Sensitive actions like access control changes and ownership on contracts should emit events for easy tracking and notification.

Suggested course of action

Implement new events for functions that should have them.

Low

Unorthodox token decimals normalization algorithm limits maximum token precision and introduces operational risk

Locations: 1 2

Description

The Oracle contract handles the recording of pricing data for the lending system. Other contracts utilize viewPrice() and getPrice() to fetch pricing data.

These functions include the following lines:

uint8 feedDecimals = feeds[token].feed.decimals();
uint8 tokenDecimals = feeds[token].tokenDecimals;
uint8 decimals = 36 - feedDecimals - tokenDecimals;
uint normalizedPrice = price * (10 ** decimals);

tokenDecimals comes from data supplied to the setter function setFeed() as an argument set by the operator. feedDecimals comes from data supplied by the Chainlink infrastructure.

As such there are a couple of potential issues.

1) It's unclear from the code why two 'samples' of the token's precision are being taken during normalization. If the operator supplies an incorrect value via setFeed(), the normalized price could be off by one or more orders of magnitude. The gain compared to the operational risk is unclear. 1) Due to the subtraction, assuming the two values are equal, the protocol is limiting itself to tokens that use at most 18 decimals of precision. While this is the overwhelming majority of tokens, this makes the contract somewhat inflexible against future developments.

Suggested course of action

Consider the business case for allowing the operator to supply a token precision that diverges from the Chainlink-supplied value. If the intention is to tacitly support other oracles that might not directly supply precision values, carefully weigh the risk of such oracles later adding such data to the operation of the protocol oracle.

Low

DBR token lacks address(0) guards on transfer functions while possessing public burn function

Description

​ The DolaBorrowingRights contract implements most ERC20 semantics, including transfer functions and mint/burn functions. ​ However, while burn() is public and emits an Event for burning, the transfer() and transferFrom() functions lack a check to block 'burning' of tokens by sending them to the 0 address. ​

Suggested course of action

​ Modify the transfer functions to that it's clear that burning tokens is handled via burn() and so that the correct intentioned events are emitted.

c4-judge commented 1 year ago

0xean marked the issue as grade-b

c4-judge commented 1 year ago

0xean marked the issue as grade-a