dacian - Borrower can't repay but can be liquidated as token whitelist can prevent existing positions from repaying

[sherlock-admin]

dacian

high

Borrower can't repay but can be liquidated as token whitelist can prevent existing positions from repaying

Summary

Borrower can't repay but can be liquidated as token whitelist can prevent existing positions from repaying.

Vulnerability Detail

BlueBerryBank.repay() has onlyWhitelistedToken() modifier, while BlueBerryBank.liquidate() does not; both end up calling _repay(). If Borrower has an existing position and then the token is removed from the whitelist, Borrower is unable to repay but can still be liquidated.

Impact

Borrower with existing position can't repay their loan but can be liquidated - this severely disadvantages the Borrower guaranteeing their liquidation with no possibility to repay.

Code Snippet

BlueBerryBank.liquidate() L487-491 vs BlueBerryBank.repay() L718-721

Tool used

Weaponized Autism (I read through every single c4/sherlock lending/borrowing contest and examined every single high/medium finding, since the beginning. Kinda crazy right?)

Recommendation

First please consider Repayments Paused While Liquidations Enabled from BlueBerry's first audit finding. BlueBerry addressed this issue by having liquidate() call isRepayAllowed() L492

However the same state can also be reached due to the inconsistent use of onlyWhitelistedToken() modifier between repay() and liquidate(). So one potential fix is to have liquidate() also use onlyWhitelistedToken() modifier, therefore at least if the Borrower can't repay, they also can't be liquidated.

Now secondly please consider Collateral Pause Stops Existing Repayment & Liquidation, a high finding from Sherlock's Isomorph Audit. In this audit it was judged that if governance disallowed a previously allowed token and if this causes existing positions to not be able to be repaid & liquidated, this was also a high finding, as governance disallowing a token should only apply to new positions, but existing positions should be allowed to continue to be repaid and liquidated, even if the token is no longer approved by governance.

So ideally neither repay() nor liquidate() would have onlyWhitelistedToken() - this is fair to all market participants and is the most consistent fix in line with the precedent set by the judging in Sherlock's Isomorph audit. I have submitted as High since that is what Sherlock's Isomorph audit classified the same bug. If my submission is downgraded to medium, kindly please explain why the same issue was High in Isomorph but is only medium here.

My submission actually combines 2 distinct issues which have been recognized separately in previous Sherlock competitions:

• Borrower can't repay but can be liquidated
• Governance token disallow prevents existing positions from repay (and in other contests from liquidation)

However because the primary goal of the audit is to benefit the sponsor, and because the ideal solution (remove onlyWhitelistedToken() from repay()) resolves both issues, I have combined them into this single issue to keep all discussion concentrated in the one place. I do hope that this won't disadvantage me in judging, and at the very least combining both issues into one submission should uphold this submission as a high finding.

[Gornutz]

Duplicate of 117

[devdacian]

Escalate for 10 USDC

Let A = issue 117, B = issue 4

A = after the attempted fix in the update contest, once repayments have been unpaused, Borrowers who became subject to liquidation during the repayment pausing can be immediately liquidated by MEV/front-running bots. Hence after repayment is unpaused there should be a grace period that allows Borrowers to repay without being subjected to liquidation. [my comment - technically the Borrower could run their own front-running repayment bot so they still have a chance to repay before liquidation, but I do believe this issue is a valid finding].

B = due to the inconsistent use of the onlyWhitelistedToken() modifier, there exists a completely separate path (that was not discovered during the initial audit contest) where repayments are never paused, but instead a previously allowed token is disallowed. This creates a state where the Borrower is prevented from repaying but can be liquidated. Here the Borrower has absolutely no possibility of beating the liquidator, they can be liquidated at any time while being prevented from repaying.

B has been marked by both sponsor & judge as a duplicate of A. Please explain in what universe does A == B? @Gornutz @ctf-sec @IAm0x52

[pauliax]

Escalate for 10 USDC
Agree that it is not a duplicate of #117, but I think this issue should be invalid because a similar issue was submitted in the first round of the Blueberry audit: "If the token is removed from the whitelist, the borrowers are risking getting liquidated without the ability to try repaying the debt first." sherlock-audit/2023-02-blueberry-judging#249
and it was marked as a duplicate of non-reward with such an explanation: sherlock-audit/2023-02-blueberry-judging#11 (comment)

[sherlock-admin]

Escalate for 10 USDC
Agree that it is not a duplicate of #117, but I think this issue should be invalid because a similar issue was submitted in the first round of the Blueberry audit: "If the token is removed from the whitelist, the borrowers are risking getting liquidated without the ability to try repaying the debt first." sherlock-audit/2023-02-blueberry-judging#249
and it was marked as a duplicate of non-reward with such an explanation: sherlock-audit/2023-02-blueberry-judging#11 (comment)

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

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

[devdacian]

Escalate for 10 USDC

@pauliax Thank you for letting me know about that; I wasn't around for the first competition and as it didn't make the report, I never saw it! Thank you for pointing this out!

This raises an interesting question though. I first learned of this vulnerability class from studying Sherlock's Isomorph audit where this issue was submitted by @IAm0x52 and was judged in that contest to be a High Finding.

So the question which naturally follows is, why was it judged a High Finding when @IAm0x52 submitted it in a previous Sherlock contest, but when I submit it, it is initially judged a medium, then claimed as a duplicate compared to MEV stuff, then claimed to not be a valid submission at all? What is the objective standard of truth that when applied to 0x52's submission judges it High, but when applied to my submission judges it medium/invalid? Arguably my submission is even more serious impact that 0x52's, as in his submission Borrowers were blocked from repay & being liquidated, but in mine Borrowers are blocked from repay but can still be liquidated.

I would also like to challenge the Sponsors argument for marking #11 in the original contest as invalid which was: "Tokens that are removed from the whitelist go through a wind-down period which should be sufficient for all users to close their position."

Where is the immutable code for this wind-down period? It doesn't appear to exist. The Sponsor is appealing to something that may or may not exist outside of the submitted code for audit - to something that no one has seen, no one can verify, to their hopes, dreams, fantasies, promises.

In contrast my argument is from the absolute objective truth: the code. What does the code allow, how can the code be used, what states can the code enter. This is something that everyone can see & everyone can verify.

The question now becomes, what is Sherlock's ultimate epistemological standard for truth? Is it the absolute truth that the code provides, or is it the hopes, dreams, fantasies & promises of the Sponsor?

I'm advocating that Sherlock's ultimate epistemological standard of truth for judging should be the absolute objective truth of the code submitted for audit, and hence my finding should be upheld as valid.

In such a case the Sponsor would have every right to mark it as "won't fix" and state as their reason their claim to this mythical "wind-down period" which may or may not exist outside the code which no one can verify, and market participants would be free to trust them or not. But market participants should be made aware by anyone auditing this protocol that the admin is able to put Borrowers into a state where they are unable to repay their loans but able to be liquidated, severely disadvantaging Borrowers.

[sherlock-admin]

Escalate for 10 USDC

@pauliax Thank you for letting me know about that; I wasn't around for the first competition and as it didn't make the report, I never saw it! Thank you for pointing this out!

This raises an interesting question though. I first learned of this vulnerability class from studying Sherlock's Isomorph audit where this issue was submitted by @IAm0x52 and was judged in that contest to be a High Finding.

So the question which naturally follows is, why was it judged a High Finding when @IAm0x52 submitted it in a previous Sherlock contest, but when I submit it, it is initially judged a medium, then claimed as a duplicate compared to MEV stuff, then claimed to not be a valid submission at all? What is the objective standard of truth that when applied to 0x52's submission judges it High, but when applied to my submission judges it medium/invalid? Arguably my submission is even more serious impact that 0x52's, as in his submission Borrowers were blocked from repay & being liquidated, but in mine Borrowers are blocked from repay but can still be liquidated.

I would also like to challenge the Sponsors argument for marking #11 in the original contest as invalid which was: "Tokens that are removed from the whitelist go through a wind-down period which should be sufficient for all users to close their position."

Where is the immutable code for this wind-down period? It doesn't appear to exist. The Sponsor is appealing to something that may or may not exist outside of the submitted code for audit - to something that no one has seen, no one can verify, to their hopes, dreams, fantasies, promises.

In contrast my argument is from the absolute objective truth: the code. What does the code allow, how can the code be used, what states can the code enter. This is something that everyone can see & everyone can verify.

The question now becomes, what is Sherlock's ultimate epistemological standard for truth? Is it the absolute truth that the code provides, or is it the hopes, dreams, fantasies & promises of the Sponsor?

I'm advocating that Sherlock's ultimate epistemological standard of truth for judging should be the absolute objective truth of the code submitted for audit, and hence my finding should be upheld as valid.

In such a case the Sponsor would have every right to mark it as "won't fix" and state as their reason their claim to this mythical "wind-down period" which may or may not exist outside the code which no one can verify, and market participants would be free to trust them or not. But market participants should be made aware by anyone auditing this protocol that the admin is able to put Borrowers into a state where they are unable to repay their loans but able to be liquidated, severely disadvantaging Borrowers.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

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

[ctf-sec]

https://docs.sherlock.xyz/audits/judging/judging

Medium: There is a viable scenario (even if unlikely) that could cause the protocol to enter a state where a material amount of funds can be lost. The attack path is possible with assumptions that either mimic on-chain conditions or reflect conditions that have a reasonable chance of becoming true in the future. The more expensive the attack is for an attacker, the less likely it will be included as a Medium (holding all other factors constant). The vulnerability must be something that is not considered an acceptable risk by a reasonable protocol team.

can be separate medium, will leave sherlock to decide

There is a viable scenario (even if unlikely) that could cause the protocol to enter a state where a material amount of funds can be lost.

[hrishibhat]

Escalation accepted

Not a duplicate of #117
However, this issue cannot be considered a valid issue on its own due to the following reasons.

1. This is an issue with a Wont fix label from the sponsor from one of the previous contests.
2. The removal of a whitelist is the admin-controlled function. Admin is trusted with these operations.

However, Sherlock acknowledges the above comments by the Watson and does not completely disagree with the some of points raised. The validity of the issue is definitely debatable and something Sherlock needs to decide on when considering Sponsor comments.
For the reasons mentioned above the state of this issue cannot be changed as this is an update contest, but would like to assure the Watson that suggestions made will definitely be considered an clearer rules will be formed around accepting Sponsor comments.

[sherlock-admin]

Escalation accepted

Not a duplicate of #117
However, this issue cannot be considered a valid issue on its own due to the following reasons.

1. This is an issue with a Wont fix label from the sponsor from one of the previous contests.
2. The removal of a whitelist is the admin-controlled function. Admin is trusted with these operations.

However, Sherlock acknowledges the above comments by the Watson and does not completely disagree with the some of points raised. The validity of the issue is definitely debatable and something Sherlock needs to decide on when considering Sponsor comments.
For the reasons mentioned above the state of this issue cannot be changed as this is an update contest, but would like to assure the Watson that suggestions made will definitely be considered an clearer rules will be formed around accepting Sponsor comments.

This issue's escalations have been accepted!

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