Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

holyhansss - Malicous user can trigger DOS in refundDeposit function #166

Closed
github-actions bot opened this issue Feb 21, 2023 · 0 comments
Closed
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue Reward A payout will be made for this issue

Comments

@github-actions
Copy link

github-actions bot commented Feb 21, 2023

holyhansss

medium

Malicous user can trigger DOS in refundDeposit function

Summary

refundDeposit function in DepositManagerV1 contract is vulnerable to DOS attack.

Vulnerability Detail

receiveFunds() in BountyCore only callable by DepositManager and there is fundBountyToken() in DepositManagerV1 which calls receiveFunds(). It requires the _volume to be greater than 0. However, when a user send protocol token(native token), they can bypass 0 amount check. If user set volume as non-zero, and msg.value as 0, it will successfully work.

Whenever fundBountyToken() is called, the length of deposits increases by 1. The variable deposits is used in getLockedFunds()which is view function. The problem comes when user what to refund deposit using refundDeposit().

Since refundDeposit() uses getLockedFunds(), and getLockedFund() uses length of deposits for loop, refundDopsit() can be DOS. View function does not cost gas, but if view function is used inside of another state changing function, it uses gas.

Even though, it is protocol token is not whitelisted, malicious user can attack the contract with DOS. If Token A with 18 decimal is only token whitelisted, malicious user can simply call fundBountyToken() with 1 wei of Token A.

Also, Ploygon’s gas fee is very cheap, malicious user can easily DOS the refundDopsit().

Ploygon gas fee is less than $0.005. It will take less than $50 to DOS the refundDopsit().

// modified test code
// this should not work, but it works.
await depositManager.fundBountyToken(bountyAddress, ethers.constants.AddressZero, volume, 1, Constants.funderUuid, { value: 0 });

Impact

Malicious user can DOS the refundDopsit() with less than $50.

Code Snippet

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/DepositManager/Implementations/DepositManagerV1.sol#L54-L56

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/BountyCore.sol#L333-L352

Tool used

VS Code

Recommendation

Recomend to limit maximum depositors.
Recomend to add minimum deposit volume.

Duplicate of #77

@github-actions github-actions bot added Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue labels Feb 21, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Mar 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

1 participant