mssql-tds: unguarded shift in DecimalParts::to_decimal_string panics (debug) or returns garbage (release) on a >4-limb decimal payload

[David-Engel]

Summary

DecimalParts::to_decimal_string reassembles the base-2^32 limbs with an unbounded shift, so a decimal / numeric payload with more than 4 limbs shifts past the width of a u128.

https://github.com/microsoft/mssql-rs/blob/main/mssql-tds/src/datatypes/decoder.rs#L2011

let u128_value = self
    .int_parts
    .iter()
    .enumerate()
    .fold(0u128, |acc, (i, &part)| {
        acc + ((part as u32 as u128) << (i * 32))
    });

At i == 4 the shift amount is 128, which is the full width of the accumulator.

Why it is reachable

read_decimal_data derives the limb count from a wire length byte and caps it at 64, not 4:

https://github.com/microsoft/mssql-rs/blob/main/mssql-tds/src/datatypes/decoder.rs#L670

#[cfg(not(fuzzing))]
const MAX_DECIMAL_INT_PARTS: u8 = 64; // SQL Server max precision is 38, which needs max ~17 int parts

SQL Server's maximum precision of 38 fits in 4 limbs, so a payload with 5 or more limbs is malformed — but it is accepted by the decoder and reaches the shift. The input is server-controlled.

Impact

• Debug builds panic with attempt to shift left with overflow. In mssql-odbc that panic unwinds through an extern "C" boundary, which aborts the process inside the Driver Manager.
• Release builds silently return a wrong value. The shift is masked to amount % 128, so limb 4 lands back at bit 0. A [1, 0, 0, 0, 1] payload yields a magnitude of 2 rather than failing.

The + in the fold is also an overflow candidate in its own right; | is sufficient because the limbs are disjoint.

Reproduction

let int_parts: Vec<i32> = vec![1, 0, 0, 0, 1];
let v = int_parts.iter().enumerate().fold(0u128, |acc, (i, &part)| {
    acc | ((part as u32 as u128).wrapping_shl((i * 32) as u32))
});
assert_eq!(v, 1); // limb 4 wrapped onto limb 0; a checked shift would overflow

Callers

to_decimal_string is reached from Display for DecimalParts, so anything that formats a decimal value hits it. In mssql-odbc that is the SQL_C_CHAR path in column_value_to_text.

PR #217 originally added a second call site on the numeric conversion path and has since been changed to reassemble the limbs directly with a > 4 guard, so that PR no longer reaches this. The character path still does, which is why this is filed separately.

Suggested fix

Bound the limb count where the value is assembled, and use |:

if self.int_parts.len() > 4 {
    // 38 digits fit in 4 limbs; anything longer cannot be represented.
    return Err(/* or the function's existing failure mode */);
}
let mag = self.int_parts.iter().enumerate().fold(0u128, |acc, (i, &p)| {
    acc | (u128::from(p as u32) << (i * 32))
});

Tightening MAX_DECIMAL_INT_PARTS from 64 to 4 in read_decimal_data would also stop the malformed payload at the wire, and is arguably the better place to reject it. Worth doing both.

Notes

Found while reviewing #217. Credit to Saurabh Singh (@saurabh500), who identified the unguarded shift and the 64-limb cap during that review.