On Tue, Nov 7, 2017 at 4:22 AM, Steffen <i...@apachelounge.com> wrote: > Building on Windows get (new) warnings: > > apr 1.6.3 > > Win32 and Win64: > time\win32\time.c(58): warning C4098: 'SystemTimeToAprExpTime': 'void' > function returning a value
What about the attached patch? We make several round trips that cannot be corrupted, and one case of untrusted input. > Only Win32: > file_io\win32\seek.c(173): warning C4244: '=': conversion from 'apr_off_t' > to 'apr_size_t', possible loss of data No flaw, could be casted, it is a delta within program heap. > Only Win64: > memory\unix\apr_pools.c(322): warning C4267: '=': conversion from 'size_t' > to 'apr_uint32_t', possible loss of data > memory\unix\apr_pools.c(422): warning C4267: '=': conversion from 'size_t' > to 'apr_uint32_t', possible loss of data > memory\unix\apr_pools.c(446): warning C4267: '=': conversion from 'size_t' > to 'apr_uint32_t', possible loss of data > memory\unix\apr_pools.c(447): warning C4267: '=': conversion from 'size_t' > to 'apr_uint32_t', possible loss of data > memory\unix\apr_pools.c(448): warning C4267: '=': conversion from 'size_t' > to 'apr_uint32_t', possible loss of data > memory\unix\apr_pools.c(873): warning C4267: '=': conversion from 'size_t' > to 'apr_uint32_t', possible loss of data > memory\unix\apr_pools.c(1289): warning C4267: '=': conversion from 'size_t' > to 'apr_uint32_t', possible loss of data > memory\unix\apr_pools.c(1445): warning C4267: '=': conversion from 'size_t' > to 'apr_uint32_t', possible loss of data The unit within the apr_allocator_t is expressed as apr_size_t, all these local apr_uint32_t's are wrong. No time to look at apr-util at this moment, but I'll revisit it later.
Index: time/win32/time.c =================================================================== --- time/win32/time.c (revision 1814503) +++ time/win32/time.c (working copy) @@ -54,9 +54,6 @@ static const int dayoffset[12] = {0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334}; - if (tm->wMonth < 1 || tm->wMonth > 12) - return APR_EBADDATE; - /* Note; the caller is responsible for filling in detailed tm_usec, * tm_gmtoff and tm_isdst data when applicable. */ @@ -111,6 +108,7 @@ FileTimeToSystemTime(&ft, &st); /* The Platform SDK documents that SYSTEMTIME/FILETIME are * generally UTC, so no timezone info needed + * The time value makes a roundtrip, st cannot be invalid below. */ SystemTimeToAprExpTime(result, &st); result->tm_usec = (apr_int32_t) (input % APR_USEC_PER_SEC); @@ -127,6 +125,7 @@ FileTimeToSystemTime(&ft, &st); /* The Platform SDK documents that SYSTEMTIME/FILETIME are * generally UTC, so we will simply note the offs used. + * The time value makes a roundtrip, st cannot be invalid below. */ SystemTimeToAprExpTime(result, &st); result->tm_usec = (apr_int32_t) (input % APR_USEC_PER_SEC); @@ -158,6 +157,7 @@ * because FileTimeToLocalFileFime is documented that the * resulting time local file time would have DST relative * to the *present* date, not the date converted. + * The time value makes a roundtrip, localst cannot be invalid below. */ SystemTimeToTzSpecificLocalTime(tz, &st, &localst); SystemTimeToAprExpTime(result, &localst); @@ -187,6 +187,7 @@ TIME_ZONE_INFORMATION tz; /* XXX: This code is simply *wrong*. The time converted will always * map to the *now current* status of daylight savings time. + * The time value makes a roundtrip, st cannot be invalid below. */ FileTimeToLocalFileTime(&ft, &localft); @@ -298,6 +299,9 @@ /* The Platform SDK documents that SYSTEMTIME/FILETIME are * generally UTC, so no timezone info needed */ + if (tm->wMonth < 1 || tm->wMonth > 12) + return APR_EBADDATE; + SystemTimeToAprExpTime(aprtime, *ostime); return APR_SUCCESS; }