# Attackathon \_ Fuel Network 33175 - \[Smart Contract - High] Sway-lib Subtract i Logic Vulnerability

Submitted on Sat Jul 13 2024 12:16:30 GMT-0400 (Atlantic Standard Time) by @Minato7namikazi for [Attackathon | Fuel Network](https://immunefi.com/bounty/fuel-network-attackathon/)

Report ID: #33175

Report type: Smart Contract

Report severity: High

Target: <https://github.com/FuelLabs/sway-libs/tree/0f47d33d6e5da25f782fc117d4be15b7b12d291b>

Impacts:

* Permanent freezing of funds

## Description

## Vulnerability Details

within sway-libs the `subtract` method of the `Subtract` trait implementation for `I8`

```

impl core::ops::Subtract for I8 {
    /// Subtract a I8 from a I8. Panics of overflow.
    fn subtract(self, other: Self) -> Self {
        let mut res = Self::new();
        if self.underlying >= Self::indent()
            && other.underlying >= Self::indent()
        {
            if self.underlying > other.underlying {
                res = Self::from_uint(self.underlying - other.underlying + Self::indent());
            } else {
                res = Self::from_uint(self.underlying - (other.underlying - Self::indent()));
            }
        } else if self.underlying >= Self::indent()
            && other.underlying < Self::indent()
        {
            res = Self::from_uint(self.underlying - Self::indent() + other.underlying);
        } else if self.underlying < Self::indent()
            && other.underlying >= Self::indent()
        {
            res = Self::from_uint(self.underlying - (other.underlying - Self::indent()));
        } else if self.underlying < Self::indent()
            && other.underlying < Self::indent()
        {
            if self.underlying < other.underlying {
                res = Self::from_uint(other.underlying - self.underlying + Self::indent());
            } else {
                res = Self::from_uint(self.underlying + other.underlying - Self::indent());
            }
        }
        res
    }
}
```

### The issue is in the last branch of the conditional statement:

```rust
else if self.underlying < Self::indent()
    && other.underlying < Self::indent()
{
    if self.underlying < other.underlying {
        res = Self::from_uint(other.underlying - self.underlying + Self::indent());
    } else {
        res = Self::from_uint(self.underlying + other.underlying - Self::indent());
    }
}
```

The problem is in the case where `self.underlying < other.underlying`. The current implementation is adding `Self::indent()` instead of subtracting it. This will result in incorrect subtraction for certain negative numbers.

### As example to trigger the bug

* 1- let's creates two `I8` values: `a` representing -2 and `b` representing -3.
* 2- let's performs the subtraction `a - b`, which should equal 1. because -2 - (-3) = 1
* and while

  a (representing -2) has an underlying value of 126 .. b (representing -3) has an underlying value of 125 .. and Self::indent() is 128

These values fall into the last branch of the Vulnerable if-else statement

```
else if self.underlying < Self::indent()
    && other.underlying < Self::indent()
{
    if self.underlying < other.underlying {
        res = Self::from_uint(other.underlying - self.underlying + Self::indent());
    } else {
        res = Self::from_uint(self.underlying + other.underlying - Self::indent());
    }
}
```

Since `126 (self.underlying) > 125 (other.underlying)`, it will use the second calculation:

```
res = Self::from_uint(self.underlying + other.underlying - Self::indent());
```

Plugging in the values:

```
res = Self::from_uint(126 + 125 - 128);
res = Self::from_uint(123);
```

Therefore, the incorrect result will have an underlying value of 123.

Check the PoC for full runnable example

### Impact is loss of funds because of

Incorrect Financial Calculations: Any function that uses subtraction with `I8` types, especially when dealing with negative numbers, could produce incorrect results. This could lead to logical errors in tokens & funds distribution or any computation that relies on accurate integer arithmetic.

## Recommendation

```rust
else if self.underlying < Self::indent()
    && other.underlying < Self::indent()
{
    if self.underlying < other.underlying {
        res = Self::from_uint(Self::indent() - (other.underlying - self.underlying));
    } else {
        res = Self::from_uint(self.underlying - other.underlying + Self::indent());
    }
}
```

This change ensures that when subtracting a larger negative number from a smaller negative number, the result is correctly calculated as a positive number.

## Proof of concept

#### Add this PoC test in the end of the i8.sw

```
#[test]
fn i8_Subtraction_PoC() {


    //  -2  - (-3) should equal 1

    //  but that isn't true here


    let a = I8::neg_from(2u8); // -2
    let b = I8::neg_from(3u8); // -3


    let result = a - b;


    // The expected underlying value for 1 is 129 (128 + 1)

    let expected = I8::from_uint(129u8);



    // Assert that the underlying values are correct (this should pass)

    assert(a.underlying() == 126u8);
    assert(b.underlying() == 125u8);



    // Assert that the result is not equal to the expected value which prove the bug

    assert(!(result == expected));



    // the result value will incorrectly calculated as 123u8 instead of 129u8 in this implementation (this should pass)

    assert(result.underlying() == 123u8);



}
```