Potentially unsafe arithmetic operations

[jkbpvsc]

Our auditing software (https://www.soteria.dev/) has been picking up many potentially risky arithmetic operations from your lib.
Not sure if these are false positives, or if the potential for error is bounded.

Logs:

=============This arithmetic operation may be UNSAFE!================
Found a potential vulnerability at line 133, column 23 in /home/runner/.cargo/git/checkouts/rust-decimal-60e1ab50c2e694f6/d14a5ef/src/ops/div.rs
The mul operation may result in overflows:

 127|        }
 128|
 129|        let mut quotient = mid64 / divisor_hi32_64;
 130|        let mut remainder = self.data[0] as u64 | ((mid64 - quotient * divisor_hi32_64) << 32);
 131|
 132|        // Do quotient * lo divisor
>133|        let product = quotient * (divisor & 0xFFFF_FFFF);
 134|        remainder = remainder.wrapping_sub(product);
 135|
 136|        // Check if we've gone negative. If so, add it back
 137|        if remainder > product.bitxor(u64::MAX) {
 138|            loop {
 139|                quotient = quotient.wrapping_sub(1);

=============This arithmetic operation may be UNSAFE!================
Found a potential vulnerability at line 96, column 36 in /home/runner/.cargo/git/checkouts/rust-decimal-60e1ab50c2e694f6/d14a5ef/src/ops/div.rs
The mul operation may result in overflows:

 90|                // We can't divide at at all so result is 0. The dividend remains untouched since
 91|                // the full amount is the remainder.
 92|                return 0;
 93|            }
 94|
 95|            let quotient = low64 / divisor;
>96|            self.set_low64(low64 - (quotient * divisor));
 97|            return quotient as u32;
 98|        }
 99|
 100|        // Do a simple check to see if the hi portion of the dividend is greater than the hi
 101|        // portion of the divisor.
 102|        let divisor_hi32 = (divisor >> 32) as u32;

=============This arithmetic operation may be UNSAFE!================
Found a potential vulnerability at line 96, column 28 in /home/runner/.cargo/git/checkouts/rust-decimal-60e1ab50c2e694f6/d14a5ef/src/ops/div.rs
The sub operation may result in underflows:

 90|                // We can't divide at at all so result is 0. The dividend remains untouched since
 91|                // the full amount is the remainder.
 92|                return 0;
 93|            }
 94|
 95|            let quotient = low64 / divisor;
>96|            self.set_low64(low64 - (quotient * divisor));
 97|            return quotient as u32;
 98|        }
 99|
 100|        // Do a simple check to see if the hi portion of the dividend is greater than the hi
 101|        // portion of the divisor.
 102|        let divisor_hi32 = (divisor >> 32) as u32;

=============This arithmetic operation may be UNSAFE!================
Found a potential vulnerability at line 167, column 9 in /home/runner/.cargo/git/checkouts/rust-decimal-60e1ab50c2e694f6/d14a5ef/src/ops/div.rs
The add operation may result in overflows:

 161|        let mut quo = (dividend / divisor_hi as u64) as u32;
 162|        let mut remainder = (dividend as u32).wrapping_sub(quo.wrapping_mul(divisor_hi));
 163|
 164|        // Compute full remainder
 165|        let mut prod1 = quo as u64 * divisor.data[0] as u64;
 166|        let mut prod2 = quo as u64 * divisor.data[1] as u64;
>167|        prod2 += prod1 >> 32;
 168|        prod1 = (prod1 & 0xFFFF_FFFF) | (prod2 << 32);
 169|        prod2 >>= 32;
 170|
 171|        let mut num = self.low64();
 172|        num = num.wrapping_sub(prod1);
 173|        remainder = remainder.wrapping_sub(prod2 as u32);

=============This arithmetic operation may be UNSAFE!================
Found a potential vulnerability at line 492, column 49 in /home/runner/.cargo/git/checkouts/rust-decimal-60e1ab50c2e694f6/d14a5ef/src/ops/div.rs
The add operation may result in overflows:

 486|                            // We round if we wrapped around
 487|                            true
 488|                        } else {
 489|                            let tmp = remainder.data[1] >> 31;
 490|                            rem_low64 <<= 1;
 491|                            remainder.set_low64(rem_low64);
>492|                            remainder.data[2] = (&remainder.data[2] << 1) + tmp;
 493|
 494|                            match remainder.data[2].cmp(&divisor.data[2]) {
 495|                                Ordering::Less => false,
 496|                                Ordering::Equal => {
 497|                                    let divisor_low64 = divisor.low64();
 498|                                    if rem_low64 > divisor_low64 {

=============This arithmetic operation may be UNSAFE!================
Found a potential vulnerability at line 256, column 23 in /home/runner/.cargo/git/checkouts/rust-decimal-60e1ab50c2e694f6/d14a5ef/src/ops/add.rs
The mul operation may result in overflows:

 250|        let power = if rescale_factor <= MAX_I32_SCALE {
 251|            POWERS_10[rescale_factor as usize] as u64
 252|        } else {
 253|            POWERS_10[9] as u64
 254|        };
 255|
>256|        let tmp_low = (low64 & U32_MASK) * power;
 257|        tmp64 = (low64 >> 32) * power + (tmp_low >> 32);
 258|        low64 = (tmp_low & U32_MASK) + (tmp64 << 32);
 259|        tmp64 >>= 32;
 260|        tmp64 += (high as u64) * power;
 261|
 262|        rescale_factor -= MAX_I32_SCALE;

=============This arithmetic operation may be UNSAFE!================
Found a potential vulnerability at line 257, column 17 in /home/runner/.cargo/git/checkouts/rust-decimal-60e1ab50c2e694f6/d14a5ef/src/ops/add.rs
The mul operation may result in overflows:

 251|            POWERS_10[rescale_factor as usize] as u64
 252|        } else {
 253|            POWERS_10[9] as u64
 254|        };
 255|
 256|        let tmp_low = (low64 & U32_MASK) * power;
>257|        tmp64 = (low64 >> 32) * power + (tmp_low >> 32);
 258|        low64 = (tmp_low & U32_MASK) + (tmp64 << 32);
 259|        tmp64 >>= 32;
 260|        tmp64 += (high as u64) * power;
 261|
 262|        rescale_factor -= MAX_I32_SCALE;
 263|

=============This arithmetic operation may be UNSAFE!================
Found a potential vulnerability at line 258, column 17 in /home/runner/.cargo/git/checkouts/rust-decimal-60e1ab50c2e694f6/d14a5ef/src/ops/add.rs
The add operation may result in overflows:

 252|        } else {
 253|            POWERS_10[9] as u64
 254|        };
 255|
 256|        let tmp_low = (low64 & U32_MASK) * power;
 257|        tmp64 = (low64 >> 32) * power + (tmp_low >> 32);
>258|        low64 = (tmp_low & U32_MASK) + (tmp64 << 32);
 259|        tmp64 >>= 32;
 260|        tmp64 += (high as u64) * power;
 261|
 262|        rescale_factor -= MAX_I32_SCALE;
 263|
 264|        if tmp64 > U32_MAX {

=============This arithmetic operation may be UNSAFE!================
Found a potential vulnerability at line 360, column 13 in /home/runner/.cargo/git/checkouts/rust-decimal-60e1ab50c2e694f6/d14a5ef/src/ops/common.rs
The div operation may result in overflows:

 354|            rescale_target = scale - MAX_PRECISION_I32;
 355|        }
 356|
 357|        if rescale_target > 0 {
 358|            // We're going to keep reducing by powers of 10. So, start by reducing the scale by
 359|            // that amount.
>360|            scale -= rescale_target;
 361|            let mut sticky = 0;
 362|            let mut remainder = 0;
 363|            loop {
 364|                sticky |= remainder;
 365|                let mut power = if rescale_target > 8 {
 366|                    POWERS_10[9]

=============This arithmetic operation may be UNSAFE!================
Found a potential vulnerability at line 373, column 36 in /home/runner/.cargo/git/checkouts/rust-decimal-60e1ab50c2e694f6/d14a5ef/src/ops/common.rs
The mul operation may result in overflows:

 367|                } else {
 368|                    POWERS_10[rescale_target as usize]
 369|                };
 370|
 371|                let high = self.data[upper];
 372|                let high_quotient = high / power;
>373|                remainder = high - high_quotient * power;
 374|
 375|                for item in self.data.iter_mut().rev().skip(6 - upper) {
 376|                    let num = (*item as u64).wrapping_add((remainder as u64) << 32);
 377|                    *item = (num / power as u64) as u32;
 378|                    remainder = (num as u32).wrapping_sub(item.wrapping_mul(power));
 379|                }

=============This arithmetic operation may be UNSAFE!================
Found a potential vulnerability at line 373, column 29 in /home/runner/.cargo/git/checkouts/rust-decimal-60e1ab50c2e694f6/d14a5ef/src/ops/common.rs
The sub operation may result in underflows:

 367|                } else {
 368|                    POWERS_10[rescale_target as usize]
 369|                };
 370|
 371|                let high = self.data[upper];
 372|                let high_quotient = high / power;
>373|                remainder = high - high_quotient * power;
 374|
 375|                for item in self.data.iter_mut().rev().skip(6 - upper) {
 376|                    let num = (*item as u64).wrapping_add((remainder as u64) << 32);
 377|                    *item = (num / power as u64) as u32;
 378|                    remainder = (num as u32).wrapping_sub(item.wrapping_mul(power));
 379|                }

=============This arithmetic operation may be UNSAFE!================
Found a potential vulnerability at line 234, column 27 in /home/runner/.cargo/git/checkouts/rust-decimal-60e1ab50c2e694f6/d14a5ef/src/ops/add.rs
The mul operation may result in overflows:

 228|            let power = if rescale_factor <= MAX_I32_SCALE {
 229|                POWERS_10[rescale_factor as usize] as u64
 230|            } else {
 231|                POWERS_10[9] as u64
 232|            };
 233|
>234|            let tmp_low = (low64 & U32_MASK) * power;
 235|            let tmp_hi = (low64 >> 32) * power + (tmp_low >> 32);
 236|            low64 = (tmp_low & U32_MASK) + (tmp_hi << 32);
 237|            high = (tmp_hi >> 32) as u32;
 238|            rescale_factor -= MAX_I32_SCALE;
 239|            if rescale_factor <= 0 {
 240|                lhs.low64 = low64;

=============This arithmetic operation may be UNSAFE!================
Found a potential vulnerability at line 235, column 26 in /home/runner/.cargo/git/checkouts/rust-decimal-60e1ab50c2e694f6/d14a5ef/src/ops/add.rs
The mul operation may result in overflows:

 229|                POWERS_10[rescale_factor as usize] as u64
 230|            } else {
 231|                POWERS_10[9] as u64
 232|            };
 233|
 234|            let tmp_low = (low64 & U32_MASK) * power;
>235|            let tmp_hi = (low64 >> 32) * power + (tmp_low >> 32);
 236|            low64 = (tmp_low & U32_MASK) + (tmp_hi << 32);
 237|            high = (tmp_hi >> 32) as u32;
 238|            rescale_factor -= MAX_I32_SCALE;
 239|            if rescale_factor <= 0 {
 240|                lhs.low64 = low64;
 241|                lhs.hi = high;

=============This arithmetic operation may be UNSAFE!================
Found a potential vulnerability at line 236, column 21 in /home/runner/.cargo/git/checkouts/rust-decimal-60e1ab50c2e694f6/d14a5ef/src/ops/add.rs
The add operation may result in overflows:

 230|            } else {
 231|                POWERS_10[9] as u64
 232|            };
 233|
 234|            let tmp_low = (low64 & U32_MASK) * power;
 235|            let tmp_hi = (low64 >> 32) * power + (tmp_low >> 32);
>236|            low64 = (tmp_low & U32_MASK) + (tmp_hi << 32);
 237|            high = (tmp_hi >> - ▝ [00m:00s] Building Static Happens-Before Graph          
 32) as u32;
 238|            rescale_factor -= MAX_I32_SCALE;
 239|            if rescale_factor <= 0 {
 240|                lhs.low64 = low64;
 241|                lhs.hi = high;
 242|                return aligned_add(lhs, rhs, negative, scale, subtract);

=============This arithmetic operation may be UNSAFE!================
Found a potential vulnerability at line 12, column 21 in /home/runner/.cargo/git/checkouts/rust-decimal-60e1ab50c2e694f6/d14a5ef/src/ops/mul.rs
The add operation may result in overflows:

 6|    if d1.is_zero() || d2.is_zero() {
 7|        // We should think about this - does zero need to maintain precision? This treats it like
 8|        // an absolute which I think is ok, especially since we have is_zero() functions etc.
 9|        return CalculationResult::Ok(Decimal::ZERO);
 10|    }
 11|
>12|    let mut scale = d1.scale() + d2.scale();
 13|    let negative = d1.is_sign_negative() ^ d2.is_sign_negative();
 14|    let mut product = Buf24::zero();
 15|
 16|    // See if we can optimize this calculation depending on whether the hi bits are set
 17|    if d1.hi() | d1.mid() == 0 {
 18|        if d2.hi() | d2.mid() == 0 {

=============This arithmetic operation may be UNSAFE!================
Found a potential vulnerability at line 33, column 41 in /home/runner/.cargo/git/checkouts/rust-decimal-60e1ab50c2e694f6/d14a5ef/src/ops/mul.rs
The mul operation may result in overflows:

 27|                }
 28|
 29|                scale -= MAX_PRECISION_U32 + 1;
 30|                let mut power = BIG_POWERS_10[scale as usize];
 31|
 32|                let tmp = low64 / power;
>33|                let remainder = low64 - tmp * power;
 34|                low64 = tmp;
 35|
 36|                // Round the result. Since the divisor was a power of 10, it's always even.
 37|                power >>= 1;
 38|                if remainder >= power && (remainder > power || (low64 as u32 & 1) > 0) {
 39|                    low64 += 1;

=============This arithmetic operation may be UNSAFE!================
Found a potential vulnerability at line 33, column 33 in /home/runner/.cargo/git/checkouts/rust-decimal-60e1ab50c2e694f6/d14a5ef/src/ops/mul.rs
The sub operation may result in underflows:

 27|                }
 28|
 29|                scale -= MAX_PRECISION_U32 + 1;
 30|                let mut power = BIG_POWERS_10[scale as usize];
 31|
 32|                let tmp = low64 / power;
>33|                let remainder = low64 - tmp * power;
 34|                low64 = tmp;
 35|
 36|                // Round the result. Since the divisor was a power of 10, it's always even.
 37|                power >>= 1;
 38|                if remainder >= power && (remainder > power || (low64 as u32 & 1) > 0) {
 39|                    low64 += 1;

=============This arithmetic operation may be UNSAFE!================
Found a potential vulnerability at line 82, column 25 in /home/runner/.cargo/git/checkouts/rust-decimal-60e1ab50c2e694f6/d14a5ef/src/ops/cmp.rs
The mul operation may result in overflows:

 76|    loop {
 77|        let power = if diff >= MAX_I32_SCALE {
 78|            POWERS_10[9]
 79|        } else {
 80|            POWERS_10[diff as usize]
 81|        } as u64;
>82|        let tmp_lo_32 = (*low64 & U32_MASK) * power;
 83|        let mut tmp = (*low64 >> 32) * power + (tmp_lo_32 >> 32);
 84|        *low64 = (tmp_lo_32 & U32_MASK) + (tmp << 32);
 85|        tmp >>= 32;
 86|        tmp = tmp.wrapping_add((*high as u64) * power);
 87|        // Indicates > 96 bits
 88|        if tmp > U32_MAX {

=============This arithmetic operation may be UNSAFE!================
Found a potential vulnerability at line 83, column 23 in /home/runner/.cargo/git/checkouts/rust-decimal-60e1ab50c2e694f6/d14a5ef/src/ops/cmp.rs
The mul operation may result in overflows:

 77|        let power = if diff >= MAX_I32_SCALE {
 78|            POWERS_10[9]
 79|        } else {
 80|            POWERS_10[diff as usize]
 81|        } as u64;
 82|        let tmp_lo_32 = (*low64 & U32_MASK) * power;
>83|        let mut tmp = (*low64 >> 32) * power + (tmp_lo_32 >> 32);
 84|        *low64 = (tmp_lo_32 & U32_MASK) + (tmp << 32);
 85|        tmp >>= 32;
 86|        tmp = tmp.wrapping_add((*high as u64) * power);
 87|        // Indicates > 96 bits
 88|        if tmp > U32_MAX {
 89|            return false;

=============This arithmetic operation may be UNSAFE!================
Found a potential vulnerability at line 84, column 18 in /home/runner/.cargo/git/checkouts/rust-decimal-60e1ab50c2e694f6/d14a5ef/src/ops/cmp.rs
The add operation may result in overflows:

 78|            POWERS_10[9]
 79|        } else {
 80|            POWERS_10[diff as usize]
 81|        } as u64;
 82|        let tmp_lo_32 = (*low64 & U32_MASK) * power;
 83|        let mut tmp = (*low64 >> 32) * power + (tmp_lo_32 >> 32);
>84|        *low64 = (tmp_lo_32 & U32_MASK) + (tmp << 32);
 85|        tmp >>= 32;
 86|        tmp = tmp.wrapping_add((*high as u64) * power);
 87|        // Indicates > 96 bits
 88|        if tmp > U32_MAX {
 89|            return false;
 90|        }

[paupino]

Thank you for this! I'll look into these in a bit more detail over the next few days. I see that it is quite sensitive (e.g. we make some assumptions about two 32 bit numbers comfortably fitting into a 64 bit number) however that being said, it certainly isn't a bad thing to go through each of these alerts and use an explicit function and/or comment appropriately. I think this will be a good exercise to go through so thank you for providing this audit!

[c410-f3r]

Looks like some Solana devs are creating programs with rust_decimal.

All "high-level" arithmetic operations are currently being exercised so in practice an overflow/underflow/divivion by zero shouldn't occur. Nevertheless, a manual audit of critical/individual functions is indeed a nice thing.

Heh, it seems that you will have a laborious task ahead @paupino :)

[0xdal]

any news on this one? is it safe to use for solana contracts?