Skip to content
This repository has been archived by the owner on Apr 28, 2024. It is now read-only.

AuditorPraise - old borrowing key is used instead of newBorrowingKey when adding old loans to the newBorrowing in LiquidityBorrowingManager.takeOverDebt() #53

Open
sherlock-admin opened this issue Oct 23, 2023 · 3 comments
Assignees
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability High A valid High severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Oct 23, 2023

AuditorPraise

medium

old borrowing key is used instead of newBorrowingKey when adding old loans to the newBorrowing in LiquidityBorrowingManager.takeOverDebt()

Summary

when _addKeysAndLoansInfo() is called within LiquidityBorrowingManager.takeOverDebt(), old Borrowing Key is used and not newBorrowingKey see here

Vulnerability Detail

The old borrowing key credentials are deleted in _removeKeysAndClearStorage(oldBorrowing.borrower, borrowingKey, oldLoans); see here

And a new borrowing key is created with the holdToken, saleToken, and the address of the user who wants to take over the borrowing in the _initOrUpdateBorrowing(). see here

now the old borrowing key whose credentials are already deleted is used to update the old loans in _addKeysAndLoansInfo() instead of the newBorrowingKey generated in _initOrUpdateBorrowing() see here

Impact

wrong borrowing Key is used (i.e the old borrowing key) when adding old loans to newBorrowing

Therefore the wrong borrowing key (i.e the old borrowing key) will be added as borrowing key for tokenId of old Loans in tokenIdToBorrowingKeys in _addKeysAndLoansInfo()

(i.e when the bug of update bool being false, is corrected, devs should understand :))

Code Snippet

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

Tool used

Manual Review

Recommendation

use newBorrowingKey when calling _addKeysAndLoansInfo() instead of old borrowing key.

@github-actions github-actions bot added High A valid High severity issue Has Duplicates A valid issue with 1+ other issues describing the same vulnerability labels Oct 26, 2023
@fann95
Copy link

fann95 commented Oct 29, 2023

This is an inattention error related to the accelerated development of the project. I think we would have fixed this during testing, but thanks for the hint anyway. We will fix it.

@fann95 fann95 added Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed labels Oct 29, 2023
@fann95 fann95 self-assigned this Oct 29, 2023
@sherlock-admin sherlock-admin changed the title Ancient Daffodil Caribou - old borrowing key is used instead of newBorrowingKey when adding old loans to the newBorrowing in LiquidityBorrowingManager.takeOverDebt() AuditorPraise - old borrowing key is used instead of newBorrowingKey when adding old loans to the newBorrowing in LiquidityBorrowingManager.takeOverDebt() Oct 30, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Oct 30, 2023
@fann95
Copy link

fann95 commented Oct 31, 2023

Fixed: RealWagmi/wagmi-leverage@f5860c8

@IAm0x52
Copy link

IAm0x52 commented Nov 17, 2023

Fix looks good. Correct variable is used now

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability High A valid High severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

3 participants