sherlock-audit / 2023-02-openq-judging

4 stars 0 forks source link

joestakey - funder can lose native token in `receiveFunds` #470

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

joestakey

medium

funder can lose native token in receiveFunds

Summary

Lack of sufficient checks in receiveFunds when transferring an ERC20 token means a funder can lose native token if msg.value is mistakenly passed in the function call.

Vulnerability Detail

receiveFunds allows a funder to transfer tokens to a bounty. It separates two cases: if funding is done with MATIC, and if it is done with ERC20 tokens.

File: contracts/Bounty/Implementations/BountyCore.sol
41:         if (_tokenAddress == address(0)) {
42:             volumeReceived = msg.value;
43:         } else {
44:             volumeReceived = _receiveERC20(_tokenAddress, _funder, _volume);
45:         }

The issue is that in the ERC20 token branch (l44), it does not check msg.value == 0. If the funder mistakenly passed a non-zero msg.value, they will never be able to retrieve this amount.

Impact

The funder loses that amount of MATIC. Calling refundDeposit will not allow them to retrieve it, as the tokenAddress[_depositId] will be the ERC20 token address, not address(0).

Code Snippet

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

Tool used

Manual Review

Recommendation

File: contracts/Bounty/Implementations/BountyCore.sol
41:         if (_tokenAddress == address(0)) {
42:             volumeReceived = msg.value;
43:         } else {
+               require(msg.value == 0, "no MATIC allowed for ERC20 transfers");
44:             volumeReceived = _receiveERC20(_tokenAddress, _funder, _volume);
45:         }

Duplicate of #288