code-423n4 / 2022-05-factorydao-findings

1 stars 1 forks source link

Upgraded Q -> H from 285 [1655952312863] #290

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Judge has assessed an item in Issue #285 as High risk. The relevant finding follows:

1. Excess ether sent to FixedPricePassThruGate is lost (low)

passThruGate() redirects to a beneficiary only gate.ethCost, requiring that msg.value >= gate.ethCost. As there are no other ways to access native tokens held by this contract, the cumulative excess value, a sum of msg.value - gate.ethCost, will be permanently frozen within the contract.

Proof of Concept

Only gate.ethCost is now forwarded:

https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/FixedPricePassThruGate.sol#L53-L53

(bool sent, bytes memory data) = gate.beneficiary.call{value: gate.ethCost}("");

Recommended Mitigation Steps

Pass-though the whole msg.value as the excess is not used:

(bool sent, bytes memory data) = gate.beneficiary.call{value: msg.value}("");
gititGoro commented 2 years ago

duplicate of #48