Transferring Fee To The Fee Receiver Logic Does Not Handle Decimals Properly

[code423n4]

Lines of code

https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/FixedPrice.sol#L109

Vulnerability details

Impact

The logic https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/FixedPrice.sol#L109 might result in the fee Receiver receiving lesser eth(very little value , in wei) . Decimals values should be handles correctly.

Proof of Concept

Let's say the address(this).value value is a value in wei that ends with 25(or something that is not divisible by 20) , so a 18 digit number with last digits 25. Now according to the logic https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/FixedPrice.sol#L109 the fee is calculated by ISaleFactory(factory).feeReceiver().transfer(address(this).balance / 20); and dividing the address(this).value with 20 will give a value in decimals and the decimal values would be truncated and the fee receiver would receive lesser fees.

Tools Used

Manual analysis

Recommended Mitigation Steps

Handle the decimal values correctly , using some decimal libraries like openzepplin's one.

[c4-judge]

berndartmueller marked the issue as unsatisfactory: Insufficient proof

[c4-judge]

berndartmueller marked the issue as satisfactory

[c4-judge]

berndartmueller marked the issue as primary issue

[c4-sponsor]

stevennevins marked the issue as sponsor disputed

[stevennevins]

Rounding loses negligible amount of wei and it's to the escher fee receiver not a user

[berndartmueller]

Due to the max loss of 1 wei for the fee receiver (which is typically the protocol), I am downgrading this finding to QA (Low).

[c4-judge]

berndartmueller changed the severity to QA (Quality Assurance)

[c4-judge]

berndartmueller marked the issue as grade-c