Unidata / UDUNITS-2

API and utility for arithmetic manipulation of units of physical quantities
http://www.unidata.ucar.edu/software/udunits
Other
62 stars 36 forks source link

ut_encode_clock: signed integer overflow #92

Closed schwehr closed 3 years ago

schwehr commented 4 years ago

To report a non-security related issue, please provide:

8fb0b21826a9abcd36e097bd1cb8c95c947b9526

linux custom

third_party/udunits/lib/unitcore.c:412:18: runtime error: signed integer overflow: -2147483648 * 60 cannot be represented in type 'int'
    #0 0x5556ec8c9b7a in ut_encode_clock third_party/udunits/lib/unitcore.c:412:18
    #1 0x5556ec8c401d in decodeClock scanner.l:84:12
    #2 0x5556ec8c28df in utlex scanner.l:281:19
    #3 0x5556ec8bc2b3 in utparse parser.c:1388:16
    #4 0x5556ec8bed8f in ut_parse parser.y:656:17
    #5 0x5556ec8b0e1d in LLVMFuzzerTestOneInput third_party/udunits/fuzzers/ut_parse_fuzzer.cc:27:19

testcase-5704996435787776.zip

8@3-73
2147483648:0

The fix:

double
ut_encode_clock(
    int     hours,
    int     minutes,
    double  seconds)
{
    if (hours >= 24 || minutes >= 60 || seconds > 62 ||
        hours < 0 || minutes < 0 || seconds < 0) {
        ut_set_status(UT_BAD_ARG);
        return 0;
    }

    return (hours*60 + minutes)*60 + seconds;
}

I can make a pr if the fix is acceptable.

semmerson commented 3 years ago

Kurt, please make a PR. I need the practice.

schwehr commented 3 years ago

When I add the checks, it breaks this test with ut_encode_clock called with -1 0 -0.000000

        unit = ut_parse(unitSystem,
                "second since 1970-01-01 0 -1", UT_ASCII);
        CU_ASSERT_PTR_NOT_NULL(unit);
        if (unit) {
            converter = ut_get_converter(unit, secondsSinceTheEpoch);
            CU_ASSERT_PTR_NOT_NULL(converter);
            if (converter) {
                CU_ASSERT_TRUE(areCloseDoubles(cv_convert_double(converter, 0), 3600));
                cv_free(converter);
            }
            ut_free(unit);
        }
udunits2 
You have: second since 1970-01-01 0 -1
You want: second since 1970-01-01 0 0 
    1 second since 1970-01-01 0 -1 = 3601 (second since 1970-01-01 0 0)
    x/(second since 1970-01-01 0 0) = (x/(second since 1970-01-01 0 -1)) + 3600

So what should ut_encode_clock be doing? Should hour be > -24? min > -60 and sec >= -62?

schwehr commented 3 years ago

This was fixed in https://github.com/Unidata/UDUNITS-2/commit/5d956f7624fff8080679b9f80fe80d7ae043f8c8 with the abs and fabs:

    if (abs(hours) >= 24 || abs(minutes) >= 60 || fabs(seconds) > 62) {
        ut_set_status(UT_BAD_ARG);
        return 0;
    }