code-423n4 / 2021-12-amun-findings

0 stars 0 forks source link

Redundant check on feeBeneficiaryShare #218

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

0xabcc

Vulnerability details

Impact

In BasketFacet.sol, feeBeneficiaryShare is checked in L178 and L223. However, given the condition check from L172-L174 & L217-L219, variables feeAmount and entryFeeBeneficiaryShare/exitFeeBeneficiaryShare are non-zero integers. Checking in L178 & L223 become redundant.

Proof of Concept

L222 feeBeneficiaryShare = feeAmount.mul(bs.exitFeeBeneficiaryShare).div(10**18) when feeAmount !=0 && exitFeeBeneficiaryShare!=0, feeBeneficiaryShare must be non-zero.

Tools Used

Recommended Mitigation Steps

Simply take out the check.

0xleastwood commented 2 years ago

It's possible that uint256 feeBeneficiaryShare = feeAmount.mul(bs.entryFeeBeneficiaryShare).div(10**18) actually truncates the result, so I think this is a valid check.