Skip to content

offset_to_sec skips its range check for integral Rationals, so DateTime accepts a 48-hour offset (and wraps the sign at 2**31) #181

Description

@sh4d0byss

Every branch of offset_to_sec range-checks the resulting number of seconds, except one path through the Rational branch. An integral Rational therefore reaches *rof unchecked, and is then narrowed from long to int.

require'date'DateTime.new(2024,1,1,0,0,0,2).iso8601# => "2024-01-01T00:00:00+00:00"DateTime.new(2024,1,1,0,0,0,Rational(2,1)).iso8601# => "2024-01-01T00:00:00+48:00"DateTime.new(2024,1,1,0,0,0,Rational(49710,1)).iso8601# => "2024-01-01T00:00:00-06:28"

The Integer 2 is rejected and falls back to +00:00, which is the documented behaviour (only -1, 0 and 1 are accepted as day fractions). Rational(2, 1) is the same quantity and produces a 48-hour offset. Rational(49710, 1) is 4_294_944_000 seconds, which exceeds INT_MAX; the (int) cast wraps it to a negative value, so a large positive offset comes out as a negative one.

Cause: ext/date/date_core.c, offset_to_sec (around :2600). The range check sits inside the else arm:

if (FIXNUM_P(vn) &&FIXNUM_P(vd) && (FIX2LONG(vd) ==1))
n=FIX2LONG(vn); /* integral: no range check */else {
vn=f_round(vs);
...
n=FIX2LONG(vn);
if (n<-DAY_IN_SECONDS||n>DAY_IN_SECONDS) /* only here */return0;
}
*rof= (int)n; /* long -> int */

The T_FIXNUM, T_FLOAT and T_STRING branches all check before assigning. Moving the check below the if/else, so it covers both arms, would make the Rational path consistent with them and also remove the narrowing problem.

Related history: #74 and #77 added and then repositioned the equivalent element checks for the parsing path in date_parse.c after an OSS-Fuzz report. This is the constructor path, which kept the check only in one arm since 7ab3f98604.

Tested: ruby 3.4.5 / date 3.5.1, and ruby 4.1.0dev (master) / date 3.5.1. Same result on both.

Filing publicly as a correctness bug: the effect is an out-of-range value being accepted and formatted, with 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