Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential failure in liquidation process due to blacklisted recipients #48

Closed
howlbot-integration bot opened this issue Jun 17, 2024 · 3 comments
Closed
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-42 edited-by-warden partial-75 Incomplete articulation of vulnerability; eligible for partial credit only (75%) 🤖_27_group AI based duplicate group recommendation sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sufficient quality report This report is of sufficient quality

Comments

@howlbot-integration
Copy link

Lines of code

https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/libraries/logic/LiquidationLogic.sol#L93-L100

Vulnerability details

Impact

According to the README.md, amongst the ERC20 tokens used by the protocol are USDC and USDT.

In the LiquidationLogic library, the liquidate function attempts to transfer the remaining margin to the vault recipient using the safeTransfer method.

If the recipient is blacklisted, as can happen with USDC and USDT, this transfer will fail, potentially halting the liquidation process.

Proof of Concept

The liquidation process relies on the safeTransfer function to transfer funds to the vault recipient.

                if (vault.recipient != address(0)) {
                    // Send the remaining margin to the recipient.
                    vault.margin = 0;


                    sentMarginAmount = uint256(remainingMargin);


                    ERC20(pairStatus.quotePool.token).safeTransfer(vault.recipient, sentMarginAmount);
                }

The safeTransfer method will revert the transaction if the recipient is blacklisted, causing the entire liquidation process to fail.

The disruption of the liquidation process will potentially leave the vault in an under-collateralized state and expose the protocol to financial risk.

Tools Used

Manual review

Recommended Mitigation Steps

The contract should implement a check to verify whether the recipient address is blacklisted before attempting the safeTransfer.

If the recipient is blacklisted, the contract could redirect the funds to a predefined backup address.

This ensures that the liquidation process can proceed smoothly despite the blacklisted status of the vault.recipient.

Assessed type

Other

@howlbot-integration howlbot-integration bot added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value 🤖_27_group AI based duplicate group recommendation bug Something isn't working edited-by-warden primary issue Highest quality submission among a set of duplicates sufficient quality report This report is of sufficient quality labels Jun 17, 2024
howlbot-integration bot added a commit that referenced this issue Jun 17, 2024
@syuhei176
Copy link

same as #47

@syuhei176 syuhei176 added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Jun 19, 2024
@c4-judge
Copy link
Contributor

alex-ppg marked issue #42 as primary and marked this issue as a duplicate of 42

@c4-judge c4-judge added duplicate-42 and removed primary issue Highest quality submission among a set of duplicates labels Jun 28, 2024
@c4-judge
Copy link
Contributor

alex-ppg marked the issue as partial-75

@c4-judge c4-judge added the partial-75 Incomplete articulation of vulnerability; eligible for partial credit only (75%) label Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-42 edited-by-warden partial-75 Incomplete articulation of vulnerability; eligible for partial credit only (75%) 🤖_27_group AI based duplicate group recommendation sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

2 participants