`Uuid::new_v7` mishandles `usable_counter_bits` at `0` and `126,127,128` #871

New issue

Closed

opened 2026-03-15 12:21:46 -05:00 by meng-xu-cs · 1 comment

meng-xu-cs commented

2026-03-15 12:21:46 -05:00

(Migrated from github.com)

Summary

Uuid::new_v7 uses u128::overflowing_shr / overflowing_shl to merge the caller-supplied v7 counter into the UUID tail. Those operations mask shift counts modulo 128, so the edge widths are handled incorrectly.

This shows up in two ways:

usable_counter_bits = 0: the counter should contribute no bits, but a non-zero counter is ORed into the UUID tail.
usable_counter_bits in 126..=128: widths above 12 are internally bumped by +2 to skip the RFC 4122 variant bits, so the high-end range reaches shift counts 128..=130. Those wrap modulo 128, so random bits are not fully cleared and repeated calls with the same full-width counter can still differ.

Reproduction

use uuid::{Timestamp, Uuid};

fn main() {
    let seconds = 1_700_000_000;
    let nanos = 0;

    // Case 1: zero-width counter should be ignored.
    // Instead, the non-zero counter is ORed directly into the tail.
    let zero_width = Timestamp::from_unix_time(seconds, nanos, u128::MAX, 0);
    let zero_a = Uuid::new_v7(zero_width);
    let zero_b = Uuid::new_v7(zero_width);

    println!("zero_a = {zero_a}");
    println!("zero_b = {zero_b}");
    // Actual today: both calls collapse to the same all-ones tail:
    // 018bcfe5-6800-7fff-bfff-ffffffffffff

    // Case 2: full-width counter should fully determine the tail.
    let full_width = Timestamp::from_unix_time(seconds, nanos, 0, 128);
    let full_a = Uuid::new_v7(full_width);
    let full_b = Uuid::new_v7(full_width);

    println!("full_a = {full_a}");
    println!("full_b = {full_b}");
    println!("full_a == full_b: {}", full_a == full_b);

    // Expected here:
    // 018bcfe5-6800-7000-8000-000000000000
    // and repeated calls should stay identical.
    // Actual: repeated calls still differ because random bits leak into the tail.
}

Uuid::new_v7 currently does:

if counter_bits > 12 {
    let mask = u128::MAX << (counter_bits - 12);
    counter = (counter & !mask) | ((counter & mask) << 2);
    counter_bits += 2;
}

counter_and_random &= u128::MAX.overflowing_shr(counter_bits).0;
counter_and_random |= counter
    .overflowing_shl(128u32.saturating_sub(counter_bits))
    .0;

Because overflowing_shr / overflowing_shl mask the shift amount modulo 128:

usable_counter_bits = 0 ends up doing counter.overflowing_shl(128), which behaves like a shift by 0, so the counter is ORed into the tail instead of ignored.
usable_counter_bits = 126..=128 becomes an internal width of 128..=130 after the variant-field adjustment, so the random mask does not become zero and random bits leak into supposedly full-width counters.

Suggested fix

Handle the edge cases without relying on overflowing shifts. For example:

if counter_bits > 12 {
    let mask = u128::MAX << (counter_bits - 12);
    counter = (counter & !mask) | ((counter & mask) << 2);
    counter_bits += 2;
}

match counter_bits {
    0 => {
        // Leave `counter_and_random` unchanged: the counter contributes no bits.
    }
    128.. => {
        counter_and_random = 0;
        counter_and_random |= counter;
    }
    n => {
        counter_and_random &= u128::MAX >> n;
        counter_and_random |= counter << (128 - n);
    }
}

### Summary `Uuid::new_v7` uses `u128::overflowing_shr` / `overflowing_shl` to merge the caller-supplied v7 counter into the UUID tail. Those operations mask shift counts modulo 128, so the edge widths are handled incorrectly. This shows up in two ways: - `usable_counter_bits = 0`: the counter should contribute no bits, but a non-zero `counter` is ORed into the UUID tail. - `usable_counter_bits in 126..=128`: widths above 12 are internally bumped by `+2` to skip the RFC 4122 variant bits, so the high-end range reaches shift counts `128..=130`. Those wrap modulo 128, so random bits are not fully cleared and repeated calls with the same full-width counter can still differ. ### Reproduction ```rust use uuid::{Timestamp, Uuid}; fn main() { let seconds = 1_700_000_000; let nanos = 0; // Case 1: zero-width counter should be ignored. // Instead, the non-zero counter is ORed directly into the tail. let zero_width = Timestamp::from_unix_time(seconds, nanos, u128::MAX, 0); let zero_a = Uuid::new_v7(zero_width); let zero_b = Uuid::new_v7(zero_width); println!("zero_a = {zero_a}"); println!("zero_b = {zero_b}"); // Actual today: both calls collapse to the same all-ones tail: // 018bcfe5-6800-7fff-bfff-ffffffffffff // Case 2: full-width counter should fully determine the tail. let full_width = Timestamp::from_unix_time(seconds, nanos, 0, 128); let full_a = Uuid::new_v7(full_width); let full_b = Uuid::new_v7(full_width); println!("full_a = {full_a}"); println!("full_b = {full_b}"); println!("full_a == full_b: {}", full_a == full_b); // Expected here: // 018bcfe5-6800-7000-8000-000000000000 // and repeated calls should stay identical. // Actual: repeated calls still differ because random bits leak into the tail. } ``` `Uuid::new_v7` currently does: ```rust if counter_bits > 12 { let mask = u128::MAX << (counter_bits - 12); counter = (counter & !mask) | ((counter & mask) << 2); counter_bits += 2; } counter_and_random &= u128::MAX.overflowing_shr(counter_bits).0; counter_and_random |= counter .overflowing_shl(128u32.saturating_sub(counter_bits)) .0; ``` Because `overflowing_shr` / `overflowing_shl` mask the shift amount modulo 128: - `usable_counter_bits = 0` ends up doing `counter.overflowing_shl(128)`, which behaves like a shift by 0, so the counter is ORed into the tail instead of ignored. - `usable_counter_bits = 126..=128` becomes an internal width of `128..=130` after the variant-field adjustment, so the random mask does not become zero and random bits leak into supposedly full-width counters. ### Suggested fix Handle the edge cases without relying on overflowing shifts. For example: ```rust if counter_bits > 12 { let mask = u128::MAX << (counter_bits - 12); counter = (counter & !mask) | ((counter & mask) << 2); counter_bits += 2; } match counter_bits { 0 => { // Leave `counter_and_random` unchanged: the counter contributes no bits. } 128.. => { counter_and_random = 0; counter_and_random |= counter; } n => { counter_and_random &= u128::MAX >> n; counter_and_random |= counter << (128 - n); } } ```

KodrAus commented

2026-03-15 18:48:23 -05:00

(Migrated from github.com)

Thanks for the report @meng-xu-cs!

It would be good to get this fixed up. I’m on vacation right now but can look at any PRs or take a look when I’m back in a week.

We should try improve our test infrastructure around new_v7 so we can more easily assert the behavior of combining user-specified counters with internally generated random data. A simple #[cfg(test)] thread local boxed function to return a byte slice should do it.

Thanks for the report @meng-xu-cs! It would be good to get this fixed up. I’m on vacation right now but can look at any PRs or take a look when I’m back in a week. We should try improve our test infrastructure around `new_v7` so we can more easily assert the behavior of combining user-specified counters with internally generated random data. A simple `#[cfg(test)]` thread local boxed function to return a byte slice should do it.