sherlock-audit / 2023-10-real-wagmi-judging

16 stars 14 forks source link

0xMaroutis - Misuse of Old Borrowing key instead of New Key in takeOverDebt Function #182

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

0xMaroutis

medium

Misuse of Old Borrowing key instead of New Key in takeOverDebt Function

Summary

The function takeOverDebt seems to mistakenly use the borrowingKey (which is the old key) instead of the expected newBorrowingKey in the _addKeysAndLoansInfo method. This can lead to unexpected behavior and potential DoS vulnerabilities, as the platform relies on a specific key format.

Vulnerability Detail

The function takeOverDebt is responsible for managing borrowing positions. During its execution, a new borrowing position is initialized or updated using the _initOrUpdateBorrowing function, which returns a new borrowing key (newBorrowingKey). However, subsequent actions are performed using the old borrowing key (borrowingKey), leading to inconsistencies in data storage and retrieval.

Specifically, in the line:

_addKeysAndLoansInfo(newBorrowing.borrowedAmount > 0, borrowingKey, oldLoans);

The borrowingKey (old key) is used instead of the newBorrowingKey. Given that the platform expects keys in the format: borrowingKey = Keys.computeBorrowingKey(msg.sender, saleToken, holdToken); This inconsistency can lead to unexpected behavior, data corruption, or potential denial-of-service vulnerabilities.

Impact

The mismanagement of borrowing keys can lead to:

The impact is high, especially for users who can only use the front-end portal to execute the trades.

Code Snippet

https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L441

Tool used

Manual Review

Recommendation

Update the takeOverDebt function to use the newBorrowingKey in the _addKeysAndLoansInfo method :

_addKeysAndLoansInfo(newBorrowing.borrowedAmount > 0, newBorrowingKey, oldLoans);

Duplicate of #53