sherlock-audit / 2023-01-ajna-judging

1 stars 0 forks source link

Jeiwan - Quote and collateral tokens cannot be added at the maximal price #149

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

Jeiwan

medium

Quote and collateral tokens cannot be added at the maximal price

Summary

Trying add quote or collateral token in the bucket with the maximal supported price (1_004_968_987.606512354182109771e18) will always result in a revert.

Vulnerability Detail

As per the constants defined in PoolHelpers, the minimal and the maximal prices are:

uint256 constant MIN_PRICE = 99_836_282_890;
uint256 constant MAX_PRICE = 1_004_968_987.606512354182109771 * 1e18;

Also, as per the documentation of the _priceAt function, the maximal price corresponds to index 0 in the Fenwick tree:

*          MAX_PRICE : bucket index 4156,  fenwick index 0:    7388-0-3232=4156.

However, trying to add tokens at the maximal price will always result in a revert due to these checks:

  1. LenderActions.sol#L104
  2. LenderActions.sol#L137

    Impact

    User won't be able to add quote and collateral tokens at the maximal price.

    Code Snippet

    LenderActions.sol#L104 LenderActions.sol#L137 LenderActions.sol#L206-L207

    Tool used

    Manual Review

    Recommendation

    Consider loosening the check in the addCollateral and addQuoteToken functions and allowing deposits at the maximal price. Alternatively, consider updating the MAX_PRICE constant and the documentation of the _priceAt function if the maximal price should be at index 1 of the Fenwick tree.

grandizzy commented 1 year ago

documentation will be updated accordingly

hrishibhat commented 1 year ago

Agree with the Sponsor on severity. Except for the mentioned function revert upon adding 0 index there does not seem to be any loss of funds, only not able to add collateral at the max price. Considering this issue as low

EdNoepel commented 1 year ago

Found no documentation to update. Updating the constant introduced complexities we would like to avoid. As such, changing to Sponsor Confirmed.