Off-by-one error in `parse_duration` can lead to panic on user-controlled input

[meng-xu-cs]

Summary

parse_duration can panic on user-controlled input instead of returning Err(DurationError::NumberOverflow).

The problem is in src/duration.rs in add_current:

fn add_current(mut sec: u64, nsec: u64, out: &mut Duration) -> Result<(), Error> {
    let mut nsec = (out.subsec_nanos() as u64).add(nsec)?;
    if nsec > 1_000_000_000 {
        sec = sec.add(nsec / 1_000_000_000)?;
        nsec %= 1_000_000_000;
    }
    sec = out.as_secs().add(sec)?;
    *out = Duration::new(sec, nsec as u32);
    Ok(())
}

The guard should be >= 1_000_000_000, not > 1_000_000_000.

When the accumulated nanoseconds are exactly 1_000_000_000, the carry is skipped and the unnormalized value is passed to Duration::new. If the seconds field is already u64::MAX, Duration::new panics while trying to carry the extra second.

Reproduction

Minimal reproduction:

fn main() {
    let _ = humantime::parse_duration("18446744073709551615s 1000000000ns");
}

This can also be reproduced with accumulated nanos across multiple pieces:

fn main() {
    let _ = humantime::parse_duration("18446744073709551615s 999999999ns 1ns");
}

A regression test that shows the intended behavior:

#[test]
fn parse_duration_returns_error_instead_of_panicking_on_exact_nanos_carry_overflow() {
    let result = std::panic::catch_unwind(|| {
        humantime::parse_duration("18446744073709551615s 1000000000ns")
    });

    assert!(result.is_ok(), "parse_duration panicked");
    assert_eq!(
        result.unwrap(),
        Err(humantime::DurationError::NumberOverflow)
    );
}

Why this happens

For the input 18446744073709551615s 1000000000ns:

1. 18446744073709551615s sets the accumulated duration to u64::MAX seconds.
2. 1000000000ns makes the temporary nanosecond total exactly 1_000_000_000.
3. The current condition if nsec > 1_000_000_000 does not run for that exact value.
4. Duration::new(u64::MAX, 1_000_000_000) then tries to carry +1 second and panics.

Suggested fix

Change the normalization condition from > to >=:

if nsec >= 1_000_000_000 {
    sec = sec.add(nsec / 1_000_000_000)?;
    nsec %= 1_000_000_000;
}

That keeps the carry inside the existing checked-arithmetic path and turns this case into Err(Error::NumberOverflow) instead of a panic.

It would also be good to add regression tests for:

• exact carry with a single token: 18446744073709551615s 1000000000ns
• exact carry via accumulation: 18446744073709551615s 999999999ns 1ns

[djc]

Thanks for your research into issues with humantime. Unfortunately I'm winding down continued maintenance of the chrono ecosystem of crates:

• Soft-deprecating chrono and chrono-tz chrono#1768

As such, I don't have much time to spend on these issues. I might be able to review pull requests if they add test coverage and contain simple fixes.

The jiff crate offers similar functionality:

• https://docs.rs/jiff/latest/jiff/fmt/friendly/index.html

and I suppose it might benefit (more) from this level of scrutiny.

[meng-xu-cs]

Thanks for the reply! Yes, I can turn these issues into PRs for you to review. I'll also take a look at jiff.