108.md

0x52

medium

BufferBinaryOptions#isStrikeValid implements slippage incorrectly

Summary

Slippage bounds protects a user from paying a price that is too high (i.e. current price is 100 but I am willing to pay up to 101). No one would want their transaction to revert if they were paying too low of a price (i.e I am buying at the current price of 100 so only paying 99 is good). The way that slippage is implemented is that it reverts if the price is too good, which is not how slippage bounds should work.

Vulnerability Detail

function isStrikeValid(
    uint256 slippage,
    uint256 strike,
    uint256 expectedStrike
) external pure override returns (bool) {
    if (
        (strike <= (expectedStrike * (1e4 + slippage)) / 1e4) &&
        (strike >= (expectedStrike * (1e4 - slippage)) / 1e4)
    ) {
        return true;
    } else return false;
}

BufferBinaryOptions#isStrikeValid requires that the current price be within the slippage range of the expected strike. This means that if the strike price is too good then the transaction will revert.

Example: You want to open a call option with a strike price of 100. The lower the strike price the more likely your call will be ITM, so if you were to get a strike price of 99 it would be to your advantage. The higher the strike price the more likely your call will be OTM. If you were to get a strike price of 101, its less likely that your call would be ITM. In this example you need protection from the strike price going up too high not from it dropping too low. The opposite is true for a put.

Impact

Slippage protection is not implemented correctly and will revert the transaction if the strike price is too much better than expected

Code Snippet

https://github.com/sherlock-audit/2022-11-buffer/blob/main/contracts/contracts/core/BufferBinaryOptions.sol#L223-L234

Tool used

Manual Review

Recommendation

Slippage should be one-sided, only protecting against getting a worse than expected strike price:

    function isStrikeValid(
        uint256 slippage,
        uint256 strike,
        uint256 expectedStrike
+       bool isAbove
    ) external pure override returns (bool) {
        if (
-           (strike <= (expectedStrike * (1e4 + slippage)) / 1e4) &&
-           (strike >= (expectedStrike * (1e4 - slippage)) / 1e4)
+           (isAbove && strike <= (expectedStrike * (1e4 + slippage)) / 1e4) ||
+           (!isAbove && strike >= (expectedStrike * (1e4 - slippage)) / 1e4)
        ) {
            return true;
        } else return false;
    }