Snowman Merkle Airdrop
AI First Flight #10
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: high
Invalid

`s_buyFee` excessively multiplies `amount` in `buySnow()`

0xprestonong

Root

In the constructor, _buyFee is multiplied by PRECISION, contract deployers may simply pass the scaled value to _buyFee, e.g. _buyFee = 5 to represent 5 * (10 ^ PRECISION). This could lead to users thinking that s.buyFee is an additional cost on top of the amount that they intend to pay in ETH and SNOW tokens to mint.

There are two problems here:

  1. The amount argument in buySnow() must be scaled by the caller, does not follow the assumption as the _buyFee argument in the constructor.

  2. The total amount of ETH or WETH to be transferred is a product of s_buyFee and amount, instead of the sum.

Impact

The users will pay significantly higher amount in fees than the actual amount SNOW tokens to mint.

Proof of Concept

​
// FEE = 5 * PRECISION
​
function testCanBuySnow() public {
vm.startPrank(jerry);
weth.approve(address(snow), FEE);
snow.buySnow(1);
vm.stopPrank();
​
assert(weth.balanceOf(address(snow)) == FEE);
// Snow contract WETH balance: 5000000000000000000
console2.log("Snow contract WETH balance: ", weth.balanceOf(address(snow)));
assert(snow.balanceOf(jerry) == 1);
// Jerry's SNOW balance: 1
console2.log("Jerry's Snow balance: ", snow.balanceOf(jerry));
}

Mitigation / Suggested Fix:

We need a better definition for buying fees. It can either be:

  1. A percentage (or basis point) of the buy amount. OR,

  2. A fixed amount to add on top of the buy amount. (e.g. totalAmount = s.buyFee + amount

If approach 2 is favored, then it is safe to scale _buyFee in the constructor argument.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 3 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!