On Tue, Nov 7, 2017 at 4:22 AM, Steffen <[email protected]> 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;
}
