# 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: 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); } ``` --- # Agent Instructions: Querying This Documentation If you need additional information that is not directly available in this page, you can query the documentation dynamically by asking a question. Perform an HTTP GET request on the current page URL with the `ask` query parameter: ``` GET https://reports.immunefi.com/fuel-network-or-attackathon/attackathon-_-fuel-network-33175-smart-contract-high-sway-lib-subtract-i-logic-vulnerability.md?ask= ``` The question should be specific, self-contained, and written in natural language. The response will contain a direct answer to the question and relevant excerpts and sources from the documentation. Use this mechanism when the answer is not explicitly present in the current page, you need clarification or additional context, or you want to retrieve related documentation sections.