sherlock-admin3
commented
1 month ago 1 comment(s) were left on this issue during the judging contest.
tsvetanovv commented:
Only standard tokens will be used.
Closed sherlock-admin2 closed 1 month ago
sherlock-admin3
commented
1 month ago 1 comment(s) were left on this issue during the judging contest.
tsvetanovv commented:
Only standard tokens will be used.
stackbuster23
Medium
{Failure of IERC20 Token Transfer May Result in Loss of Funds for End-Users
Summary
The absence of a return value check on IERC20 transfers to users will cause a failure to credit user balances and the protocol will assume that the IERC20.transfer call succeeded without verification, whilst internal contract accounting are duly updated assuming transfer success. In the LvAssetLib.sol contract, the transfer of tokens to user addresses does not include a check to confirm whether the transfer was successful. This could lead to situations where transfers fail silently (e.g., in case of gas issues, insufficient liquidity or faulty user address), resulting in users not receiving their tokens as intended, while the protocol assumes the transfer was successful. This issue can become critical during mass transactions or stress events, causing financial loss to users and reputational damage.
Root Cause
In LvAssetLib.sol, a missing return value check in the unlockTo() function that transfers ERC20 tokens to user addresses
The function makes deduction first from user account as seen in the
decLocked functionthen does the transfer. In a situation that this fails the user does not get the token but the protocol assumes so and duly records as such. Additionally, this function is used by the PsmLib.sol library as well in function such as the _afterRedeemWithDs function, that does transfer a ra asset for the recieved pa asset the user wants.
The protocol would have gotten the user's pegged token and assumes a successful transfer of redemtion token to the user, a case of failure results in the user losing both assets.
Internal pre-conditions
User requests withdrawal or redemption of their locked tokens from the protocol.
The IERC20.transfer() function must be used directly without checking the return value.
Token transfer to user address fails (due to network congestion, insufficient gas, non-standard token behavior, or contract errors).
External pre-conditions
A liquidity crisis where the protocol does not have enough tokens available to fulfill the user’s withdrawal request.
Gas prices may spike, causing the transfer to fail if insufficient gas is provided for the transaction.
The user is interacting with a non-standard ERC20 token (e.g., one that does not return a boolean success value, like early versions of USDT).
Attack Path
The scenario is actually paths to how user loses fund whilst interacting with the protocol functions that implement this issue.
The user attempts to redeem or withdraw tokens from the protocol by calling redeemRaWithDs().
The protocol calls unlockTo() to transfer tokens to the user.
The transfer() function fails (due to one of the external pre-conditions, such as gas issues or faulty token), but the protocol does not verify the failure.
The user does not receive the tokens but the protocol assumes the transfer succeeded and updates internal state as if the transfer occurred.
The protocol could thus be under the impression that tokens were successfully withdrawn while user balances remain uncredited, leading to discrepancies and potential protocol insolvency if unchecked.
Impact
The users suffer a potential loss of tokens they are entitled to due to failed transfers not being properly detected. This can lead to:
User financial loss: Users may not receive their funds but the protocol assumes the transaction succeeded.
Reputational damage: Repeated occurrences of this issue can lead to loss of user trust in the protocol, especially in DeFi protocols that heavily depend on accurate and transparent financial transfers.
Liquidity issues: If unnoticed, the protocol may continue assuming it holds more liquidity than it actually has, leading to potential insolvency or operational failure during stress events.
No direct gain for an attacker, but the issue leads to significant user grievance and protocol liability.
Impact
The users suffer a potential loss of tokens they are entitled to due to failed transfers not being properly detected. This can lead to:
User financial loss: Users may not receive their funds but the protocol assumes the transaction succeeded.
Reputational damage: Repeated occurrences of this issue can lead to loss of user trust in the protocol, especially in DeFi protocols that heavily depend on accurate and transparent financial transfers.
Liquidity issues: If unnoticed, the protocol may continue assuming it holds more liquidity than it actually has, leading to potential insolvency or operational failure during stress events.
No direct gain for an attacker, but the issue leads to significant user grievance and protocol liability.
PoC
No response
Mitigation
To mitigate the risk of failed transfers or unexpected behavior from non-compliant ERC20 tokens, the safeTransfer function from OpenZeppelin’s SafeERC20 library should be used instead of this transfer function as used in some other functions in the protocol. The protocol must maintain a disciplined use of the SafeEr20 transfer functions for the santity of the protocol. OpenZeppelin’s safeTransfer ensures that the token contract correctly returns a success value, reverting the transaction if the transfer fails, which prevents unexpected silent failures.