sherlock-audit / 2022-10-astaria-judging

6 stars 1 forks source link

0xRajeev - `AstariaRouter.commitToLiens` will revert if the protocol fee is enabled #195

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

0xRajeev

medium

AstariaRouter.commitToLiens will revert if the protocol fee is enabled

Summary

The function commitToLiens() will revert in getProtocolFee(), which prevents borrowers from depositing collateral and requesting loans in the protocol.

Vulnerability Detail

If the protocol fee is enabled by setting feeTo to a non-zero address, then getProtocolFee() will revert because of division-by-zero given that protocolFeeDenominator is 0 without any initialization and no setter (in file()) for setting it.

Impact

The function commitToLiens() will revert if the protocol fee is enabled thus preventing borrowers from depositing collateral and requesting loans in the protocol thereby failing to bootstrap its core NFT lending functionality.

Code Snippet

  1. https://github.com/sherlock-audit/2022-10-astaria/blob/main/src/AstariaRouter.sol#L66-L67
  2. https://github.com/sherlock-audit/2022-10-astaria/blob/main/src/AstariaRouter.sol#L441-L443
  3. https://github.com/sherlock-audit/2022-10-astaria/blob/main/src/VaultImplementation.sol#L337-L340
  4. https://github.com/sherlock-audit/2022-10-astaria/blob/main/src/VaultImplementation.sol#L331
  5. https://github.com/sherlock-audit/2022-10-astaria/blob/main/src/VaultImplementation.sol#L242-L250

Tool used

Manual Review

Recommendation

Initialize protocol fee numerator and denominator in AstariaRouter and add their setters to file().

secureum commented 1 year ago

Escalate for 2 USDC.

We do not think this is a duplicate issue of #204. While both are about commitToLiens() reverting, the triggering locations, conditions and therefore the recommendations are entirely different. This issue is specific to non-initialization of protocol fee variables as described above.

cc @berndartmueller @lucyoa

sherlock-admin commented 1 year ago

Escalate for 2 USDC.

We do not think this is a duplicate issue of #204. While both are about commitToLiens() reverting, the triggering locations, conditions and therefore the recommendations are entirely different. This issue is specific to non-initialization of protocol fee variables as described above.

cc @berndartmueller @lucyoa

You've created a valid escalation for 2 USDC!

To remove the escalation from consideration: Delete your comment. To change the amount you've staked on this escalation: Edit your comment (do not create a new comment).

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

Evert0x commented 1 year ago

Escalation accepted

sherlock-admin commented 1 year ago

Escalation accepted

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.