Skip to content

Date.valid_commercial? accepts out-of-range week numbers, and Date.commercial then returns a wrong date #180

Description

@sh4d0byss

A negative week number far outside the valid range is accepted, and the corresponding Date.commercial call returns a date instead of raising.

require'date'Date.valid_commercial?(2024, -53,1)# => false (correct)Date.valid_commercial?(2024, -1227133565,1)# => true (expected false)Date.commercial(2024, -1227133565,1)# => #<Date: 2024-01-01> (expected Date::Error)

This is not a single unlucky value: sweeping w over a +-500 window around that value, 52 of the 1001 candidates are accepted.

Cause: c_valid_commercial_p (ext/date/date_core.c) computes rjd2 + w * 7 in int arithmetic with w taken straight from the caller, so w * 7 wraps. That alone would be harmless, because the function ends with a round-trip check. The problem is that the negative branch replaces the caller's value before that check runs:

if (w<0) {
intrjd2;
c_commercial_to_jd(y+1, 1, 1, sg, &rjd2, &ns2);
c_jd_to_commercial(rjd2+w*7, sg, &ry2, &rw2, &rd2); /* w * 7 wraps */if (ry2!=y)
return0;
w=rw2; /* caller value discarded */
}
c_commercial_to_jd(y, w, d, sg, rjd, ns);
c_jd_to_commercial(*rjd, sg, &ry2, rw, rd);
if (y!=ry2||w!=*rw||d!=*rd) /* compares the replaced w */return0;
return1;

So the final guard compares the normalised week against itself and passes. Because 7 is invertible modulo 2**32, values that land back inside year y can be constructed rather than found by chance.

Two independent things would each stop it: rejecting w whose magnitude cannot be meaningful (a year has at most 53 weeks) before the multiplication, and keeping the caller's w for the final comparison instead of overwriting it.

The same overflow reaches c_gregorian_civil_to_jd; note that ns_jd_in_range() guards only the jd -> civil direction, not this one.

Tested: ruby 3.4.5 / date 3.5.1, and ruby 4.1.0dev (master) / date 3.5.1. Same result on both. Ordinal and civil dates were checked too and are not affected: those functions compare against the caller's original value, and a 200k random sweep over the full int range accepted none.

Filing publicly as a correctness bug. The effect is a wrong return value from a validity predicate; there is no memory-safety impact.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions