Upgraded Q -> H from #439 [1670433195074]

Judge has assessed an item in Issue #439 as H risk. The relevant finding follows:

L02 - _close() should not be able to close a specific id credit line
As per the docs:

Can a Borrower chose to repay any debt in any order?
No. The app automatically selects which credit line can be repaid using a first-in-first-out (FIFO) repayment queue based on the order in which credit lines are drawn down
The issue is that close() takes an id argument, meaning it technically breaks this protocol requirement if called for any id other than ids[0].
Now, because of this credit.principal check, it is actually impossible to close any id other than the first one - because it is only possible to repay the full debt through depositAndClose and depositAndRepay, which always repay the first id in the array ids.

close() can hence only work for id = ids[0] anyway.

Because of the protocol desire for it to be composable, this can open risks if a child contract inheriting LineOfCredit were to give the possibility to repay full debt of any id credit line. This would mean a borrower could repay and close in any order.

Impact
Low

Tools Used
Manual Analysis

Mitigation
Modify both close() and _close(), so that they can only close the first credit line in queue

-388: function close(bytes32 id) external payable override returns (bool) {
+388: function close() external payable override returns (bool) {

•        bytes32 id = ids[0];
  

389: Credit memory credit = credits[id];
…
-406: _close(credit, id);
+406: _close(credit);
-483: function _close(Credit memory credit, bytes32 id) internal virtual returns (bool) {
+483: function _close(Credit memory credit) internal virtual returns (bool) {

•        bytes32 id = ids[0];
  

The text was updated successfully, but these errors were encountered:

All reactions