sherlock-audit / 2023-06-bond-judging

3 stars 3 forks source link

berndartmueller - Payout tokens can be stolen from the `FixedStrikeOptionTeller` contract by exercising call options without paying quote tokens #62

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

berndartmueller

high

Payout tokens can be stolen from the FixedStrikeOptionTeller contract by exercising call options without paying quote tokens

Summary

Due to a rounding error when calculating the quoteAmount in the exercise function of the FixedStrikeOptionTeller contract, it is possible to exercise call options without providing the necessary quote tokens.

Vulnerability Detail

Call option tokens can be exercised with exercise function in the FixedStrikeOptionTeller contract. However, by specifying a very small amount_, the quoteAmount calculation in line 339 can potentially round down to zero. This is the case if the result of the multiplication, $amount * strikePrice$ is smaller than $10^{decimals}$, where decimals is the number of decimals of the payout token.

For example, assume the following scenario:

Parameter Description
Quote token $USDC. 6 decimals
Payout token $GMX. 18 decimals
$payoutToken_{decimals}$ 18 decimals
$amount$ 1e10. Amount (amount_) supplied to the create function, in payout token decimal precision.
$strikePrice$ 50e6 ~ 50 USD. Strike price of the option token, in quote tokens.

Please note that the option token has the same amount of decimals as the payout token.

Calculating quoteAmount leads to the following result:

$$ \begin{align} quoteAmount &= \dfrac{amount strikePrice}{10^{payoutToken_{decimals}}} \ &= \dfrac{1e10 50e6}{10^{18}} \ &= \dfrac{5e17}{10^{18}} \ &= \dfrac{5 * 10^{17}}{10^{18}} \ &= 0 \end{align} $$

As observed, the result is rounded down to zero due to the numerator being smaller than the denominator.

This results in 0 quote tokens to be transferred from the caller (who attempts to exercise the options) to the contract, and in return, the caller receives 1e10 ($amount$) payout tokens.

Impact

Call options can be exercised for free without paying the necessary amount of quote tokens.

Code Snippet

src/fixed-strike/FixedStrikeOptionTeller.sol#L339

File: FixedStrikeOptionTeller.sol
302: function exercise(
303:     FixedStrikeOptionToken optionToken_,
304:     uint256 amount_
305: ) external override nonReentrant {
306:     // Load option parameters
...      // [...]
337:
338:     // Calculate amount of quote tokens equivalent to amount at strike price
339: @>  uint256 quoteAmount = amount_.mulDiv(strikePrice, 10 ** payoutToken.decimals()); // @audit-issue Rounds down
340:
341:     // If not receiver, require payment
342:     if (msg.sender != receiver) {
343:         // If call, transfer in quote tokens equivalent to the amount of option tokens being exercised * strike price
344:         // If put, transfer in payout tokens equivalent to the amount of option tokens being exercised
345:         if (call) {
346:             // Calculate protocol fee
347:             uint256 fee = (quoteAmount * protocolFee) / FEE_DECIMALS;
348:             fees[quoteToken] += fee;
349:
350:             // Transfer proceeds from user
351:             // Check balances before and after transfer to ensure that the correct amount was transferred
352:             // @audit this does enable potential malicious option tokens that can't be exercised
353:             // However, we view it as a "buyer beware" situation that can handled on the front-end
354:             uint256 startBalance = quoteToken.balanceOf(address(this));
355:             quoteToken.safeTransferFrom(msg.sender, address(this), quoteAmount);
356:             uint256 endBalance = quoteToken.balanceOf(address(this));
357:             if (endBalance < startBalance + quoteAmount)
358:                 revert Teller_UnsupportedToken(address(quoteToken));
359:
360:             // Transfer proceeds minus fee to receiver
361:             quoteToken.safeTransfer(receiver, quoteAmount - fee);
362:         } else {
...              // [...]
379:         }
380:     }
381:
382:     // Burn option tokens
383:     optionToken.burn(msg.sender, amount_);
384:
385:     if (call) {
386:         // Transfer payout tokens to user
387:         payoutToken.safeTransfer(msg.sender, amount_);
388:     } else {
389:         // Transfer quote tokens to user
390:         quoteToken.safeTransfer(msg.sender, quoteAmount);
391:     }
392: }

Tool used

Manual Review

Recommendation

Consider adding a check after line 339 to ensure quoteAmount is not zero.

Duplicate of #61