code-423n4 / 2021-09-wildcredit-findings

0 stars 0 forks source link

Transfer method doesn't consider gained interest correctly, #23

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

pants

Vulnerability details

Impact

transfer method doesn't consider gained interest correctly. For example, a user that gained 10% of interest and moves the LP tokens to another user might lose the gained interest. This is a severe bug and all other LP tokens out there manages the interest in a different way that supports the transfer, can be as follows:

Before transfer / deposit / withdraw of the LP you must first need to increase userGlobalReward by (globalPerTokenRewardsEarend - perTokenClaimed) * balance and then reset perTokenClaimed to be equal globalPerTokenRewardsEarend.

Since it's a missing code problem we can't add references from github.

talegift commented 2 years ago

This issue seems to be invalid.

Which contract is the warden referring to? We have no variables with such names anywhere. Even if these names are used only as an example, the calculation is completely different from how our code works.

Unlike in the previous version where interest was accrued per user, the version that's under this audit accrues interest globally.

The function responsible for LP transfers can be seen here: https://github.com/code-423n4/2021-09-wildcredit/blob/main/contracts/LendingPair.sol#L262

The transfer function is indeed not doing accruals prior to the transfer. This is intentional in order to save gas. In this case, both the sender and the recipient of LP tokens are not losing anything during the transfer, even if the accrual is not a part of the transfer.

This is because LP balances of individual accounts do not change when global accruals are performed and the transfer only changes those LP balances (not the underlying balances). Accruals only change the ratio of the underlying vs LP token.

Due to this, accruals are only needed to be executed before changing the underlying balances in some way. E.g. on deposits, withdrawals, borrowing, repayments, etc.

ghoul-sol commented 2 years ago

per sponsor comment, invalid