Re: svn commit: r1892038 - /httpd/httpd/trunk/server/util.c

2021-08-10 Thread Eric Covener
On Tue, Aug 10, 2021 at 2:52 PM Christophe JAILLET
 wrote:
>
> Le 08/08/2021 à 23:28, Eric Covener a écrit :
> > I finally got unlazy and got a local build w/ the UBISAN checking and
> > your patch works!
> >
> > env CC=clang LD=clang CFLAGS=-fsanitize=undefined ./config.nice
> >
> > Can you commit to trunk?
>
> Now tested on my side as well and pushed as r1892185.
>
> CJ

ty, just proposed.


Re: svn commit: r1892038 - /httpd/httpd/trunk/server/util.c

2021-08-10 Thread Christophe JAILLET

Le 08/08/2021 à 23:28, Eric Covener a écrit :

I finally got unlazy and got a local build w/ the UBISAN checking and
your patch works!

env CC=clang LD=clang CFLAGS=-fsanitize=undefined ./config.nice

Can you commit to trunk?


Now tested on my side as well and pushed as r1892185.

CJ





On Sun, Aug 8, 2021 at 5:03 PM Christophe JAILLET
 wrote:


Hi,

attached, my proposal. (compile tested only, but you'll get the idea)
It applies on top of current trunk (so after 1892038,1892063).

I'm not sure that the "(check > APR_INT64_MAX || check < tout)" check in
your patches is enough to catch overflows.

CJ



Re: svn commit: r1892038 - /httpd/httpd/trunk/server/util.c

2021-08-08 Thread Eric Covener
I finally got unlazy and got a local build w/ the UBISAN checking and
your patch works!

env CC=clang LD=clang CFLAGS=-fsanitize=undefined ./config.nice

Can you commit to trunk?

On Sun, Aug 8, 2021 at 5:03 PM Christophe JAILLET
 wrote:
>
> Hi,
>
> attached, my proposal. (compile tested only, but you'll get the idea)
> It applies on top of current trunk (so after 1892038,1892063).
>
> I'm not sure that the "(check > APR_INT64_MAX || check < tout)" check in
> your patches is enough to catch overflows.
>
> CJ
>
> Le 07/08/2021 à 20:33, Eric Covener a écrit :
> > Maybe making the constants unsigned?
> >
> > On Sat, Aug 7, 2021 at 2:14 PM Eric Covener  wrote:
> >>
> >> Seems like the fuzzer got farther but is still failing
> >>
> >>
> >> util.c:2713:26: runtime error: signed integer overflow:
> >>  * 1000 cannot be represented in type 'long'
> >> #0 0x4be247 in ap_timeout_parameter_parse /src/httpd/server/util.c:2713:26
> >>
> >>  case 'M':
> >>  switch (*(++time_str)) {
> >>  /* Time is in milliseconds */
> >>  case 's':
> >>  case 'S':
> >>  check = tout * 1000;
> >>  break;
> >>
> >>
> >>
> >> Does this require a cast?
> >>
> >> On Sat, Aug 7, 2021 at 6:45 AM Eric Covener  wrote:
> >>>
> >>> On Sat, Aug 7, 2021 at 3:00 AM Christophe JAILLET
> >>>  wrote:
> 
>  Hi,
> 
>  Le 06/08/2021 à 15:10, cove...@apache.org a écrit :
> > Author: covener
> > Date: Fri Aug  6 13:10:45 2021
> > New Revision: 1892038
> >
> > URL: http://svn.apache.org/viewvc?rev=1892038=rev
> > Log:
> > fix int overflow in ap_timeout_parameter_parse
> >
> > signed integer overflow in ap_timeout_parameter_parse under fuzzing
> >
> > Modified:
> >   httpd/httpd/trunk/server/util.c
> >
> > Modified: httpd/httpd/trunk/server/util.c
> > URL: 
> > http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util.c?rev=1892038=1892037=1892038=diff
> > ==
> > --- httpd/httpd/trunk/server/util.c (original)
> > +++ httpd/httpd/trunk/server/util.c Fri Aug  6 13:10:45 2021
> > @@ -2676,6 +2676,7 @@ AP_DECLARE(apr_status_t) ap_timeout_para
> >char *endp;
> >const char *time_str;
> >apr_int64_t tout;
> > +apr_uint64_t check;
> >
> >tout = apr_strtoi64(timeout_parameter, , 10);
> >if (errno) {
> > @@ -2688,16 +2689,20 @@ AP_DECLARE(apr_status_t) ap_timeout_para
> >time_str = endp;
> >}
> >
> > +if (tout < 0) {
> > +return APR_ERANGE;
> > +}
> > +
> >switch (*time_str) {
> >/* Time is in seconds */
> >case 's':
> >case 'S':
> > -*timeout = (apr_interval_time_t) apr_time_from_sec(tout);
> > +check = apr_time_from_sec(tout);
> >break;
> >case 'h':
> >case 'H':
> >/* Time is in hours */
> > -*timeout = (apr_interval_time_t) apr_time_from_sec(tout * 
> > 3600);
> > +check = apr_time_from_sec(tout * 3600);
> >break;
> >case 'm':
> >case 'M':
> > @@ -2705,12 +2710,12 @@ AP_DECLARE(apr_status_t) ap_timeout_para
> >/* Time is in milliseconds */
> >case 's':
> >case 'S':
> > -*timeout = (apr_interval_time_t) tout * 1000;
> > +check = tout * 1000;
> >break;
> >/* Time is in minutes */
> >case 'i':
> >case 'I':
> > -*timeout = (apr_interval_time_t) apr_time_from_sec(tout * 
> > 60);
> > +check = apr_time_from_sec(tout * 60);
> >break;
> >default:
> >return APR_EGENERAL;
> > @@ -2719,6 +2724,10 @@ AP_DECLARE(apr_status_t) ap_timeout_para
> >default:
> >return APR_EGENERAL;
> >}
> > +if (check > APR_INT64_MAX || check < 0) {
> 
>  why "|| check < 0"?
>  'check' is unsigned (i.e. apr_uint64_t).
> 
> >>>
> >>> ty, changed in 1892063.
> >>
> >>
> >>
> >> --
> >> Eric Covener
> >> cove...@gmail.com
> >
> >
> >
>


-- 
Eric Covener
cove...@gmail.com


Re: svn commit: r1892038 - /httpd/httpd/trunk/server/util.c

2021-08-08 Thread Christophe JAILLET

Hi,

attached, my proposal. (compile tested only, but you'll get the idea)
It applies on top of current trunk (so after 1892038,1892063).

I'm not sure that the "(check > APR_INT64_MAX || check < tout)" check in 
your patches is enough to catch overflows.


CJ

Le 07/08/2021 à 20:33, Eric Covener a écrit :

Maybe making the constants unsigned?

On Sat, Aug 7, 2021 at 2:14 PM Eric Covener  wrote:


Seems like the fuzzer got farther but is still failing


util.c:2713:26: runtime error: signed integer overflow:
 * 1000 cannot be represented in type 'long'
#0 0x4be247 in ap_timeout_parameter_parse /src/httpd/server/util.c:2713:26

 case 'M':
 switch (*(++time_str)) {
 /* Time is in milliseconds */
 case 's':
 case 'S':
 check = tout * 1000;
 break;



Does this require a cast?

On Sat, Aug 7, 2021 at 6:45 AM Eric Covener  wrote:


On Sat, Aug 7, 2021 at 3:00 AM Christophe JAILLET
 wrote:


Hi,

Le 06/08/2021 à 15:10, cove...@apache.org a écrit :

Author: covener
Date: Fri Aug  6 13:10:45 2021
New Revision: 1892038

URL: http://svn.apache.org/viewvc?rev=1892038=rev
Log:
fix int overflow in ap_timeout_parameter_parse

signed integer overflow in ap_timeout_parameter_parse under fuzzing

Modified:
  httpd/httpd/trunk/server/util.c

Modified: httpd/httpd/trunk/server/util.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util.c?rev=1892038=1892037=1892038=diff
==
--- httpd/httpd/trunk/server/util.c (original)
+++ httpd/httpd/trunk/server/util.c Fri Aug  6 13:10:45 2021
@@ -2676,6 +2676,7 @@ AP_DECLARE(apr_status_t) ap_timeout_para
   char *endp;
   const char *time_str;
   apr_int64_t tout;
+apr_uint64_t check;

   tout = apr_strtoi64(timeout_parameter, , 10);
   if (errno) {
@@ -2688,16 +2689,20 @@ AP_DECLARE(apr_status_t) ap_timeout_para
   time_str = endp;
   }

+if (tout < 0) {
+return APR_ERANGE;
+}
+
   switch (*time_str) {
   /* Time is in seconds */
   case 's':
   case 'S':
-*timeout = (apr_interval_time_t) apr_time_from_sec(tout);
+check = apr_time_from_sec(tout);
   break;
   case 'h':
   case 'H':
   /* Time is in hours */
-*timeout = (apr_interval_time_t) apr_time_from_sec(tout * 3600);
+check = apr_time_from_sec(tout * 3600);
   break;
   case 'm':
   case 'M':
@@ -2705,12 +2710,12 @@ AP_DECLARE(apr_status_t) ap_timeout_para
   /* Time is in milliseconds */
   case 's':
   case 'S':
-*timeout = (apr_interval_time_t) tout * 1000;
+check = tout * 1000;
   break;
   /* Time is in minutes */
   case 'i':
   case 'I':
-*timeout = (apr_interval_time_t) apr_time_from_sec(tout * 60);
+check = apr_time_from_sec(tout * 60);
   break;
   default:
   return APR_EGENERAL;
@@ -2719,6 +2724,10 @@ AP_DECLARE(apr_status_t) ap_timeout_para
   default:
   return APR_EGENERAL;
   }
+if (check > APR_INT64_MAX || check < 0) {


why "|| check < 0"?
'check' is unsigned (i.e. apr_uint64_t).



ty, changed in 1892063.




--
Eric Covener
cove...@gmail.com






Index: server/util.c
===
--- server/util.c	(révision 1892087)
+++ server/util.c	(copie de travail)
@@ -2668,6 +2669,7 @@ AP_DECLARE(char *) ap_append_pid(apr_pool_t *p, co
  * in timeout_parameter.
  * @return Status value indicating whether the parsing was successful or not.
  */
+#define CHECK_OVERFLOW(a, b) if (a > b) return APR_ERANGE
 AP_DECLARE(apr_status_t) ap_timeout_parameter_parse(
const char *timeout_parameter,
apr_interval_time_t *timeout,
@@ -2697,11 +2699,13 @@ AP_DECLARE(apr_status_t) ap_timeout_parameter_pars
 /* Time is in seconds */
 case 's':
 case 'S':
+CHECK_OVERFLOW(tout, apr_time_sec(APR_INT64_MAX));
 check = apr_time_from_sec(tout);
 break;
+/* Time is in hours */
 case 'h':
 case 'H':
-/* Time is in hours */
+CHECK_OVERFLOW(tout, apr_time_sec(APR_INT64_MAX / 3600));
 check = apr_time_from_sec(tout * 3600);
 break;
 case 'm':
@@ -2710,11 +2714,13 @@ AP_DECLARE(apr_status_t) ap_timeout_parameter_pars
 /* Time is in milliseconds */
 case 's':
 case 'S':
-check = tout * 1000;
+CHECK_OVERFLOW(tout, apr_time_as_msec(APR_INT64_MAX));
+check = apr_time_from_msec(tout);
 break;
 /* Time is in minutes */
 case 'i':
 case 'I':
+CHECK_OVERFLOW(tout, apr_time_sec(APR_INT64_MAX / 60));
 check = apr_time_from_sec(tout * 60);
  

Re: svn commit: r1892038 - /httpd/httpd/trunk/server/util.c

2021-08-07 Thread Eric Covener
Maybe making the constants unsigned?

On Sat, Aug 7, 2021 at 2:14 PM Eric Covener  wrote:
>
> Seems like the fuzzer got farther but is still failing
>
>
> util.c:2713:26: runtime error: signed integer overflow:
>  * 1000 cannot be represented in type 'long'
> #0 0x4be247 in ap_timeout_parameter_parse /src/httpd/server/util.c:2713:26
>
> case 'M':
> switch (*(++time_str)) {
> /* Time is in milliseconds */
> case 's':
> case 'S':
> check = tout * 1000;
> break;
>
>
>
> Does this require a cast?
>
> On Sat, Aug 7, 2021 at 6:45 AM Eric Covener  wrote:
> >
> > On Sat, Aug 7, 2021 at 3:00 AM Christophe JAILLET
> >  wrote:
> > >
> > > Hi,
> > >
> > > Le 06/08/2021 à 15:10, cove...@apache.org a écrit :
> > > > Author: covener
> > > > Date: Fri Aug  6 13:10:45 2021
> > > > New Revision: 1892038
> > > >
> > > > URL: http://svn.apache.org/viewvc?rev=1892038=rev
> > > > Log:
> > > > fix int overflow in ap_timeout_parameter_parse
> > > >
> > > > signed integer overflow in ap_timeout_parameter_parse under fuzzing
> > > >
> > > > Modified:
> > > >  httpd/httpd/trunk/server/util.c
> > > >
> > > > Modified: httpd/httpd/trunk/server/util.c
> > > > URL: 
> > > > http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util.c?rev=1892038=1892037=1892038=diff
> > > > ==
> > > > --- httpd/httpd/trunk/server/util.c (original)
> > > > +++ httpd/httpd/trunk/server/util.c Fri Aug  6 13:10:45 2021
> > > > @@ -2676,6 +2676,7 @@ AP_DECLARE(apr_status_t) ap_timeout_para
> > > >   char *endp;
> > > >   const char *time_str;
> > > >   apr_int64_t tout;
> > > > +apr_uint64_t check;
> > > >
> > > >   tout = apr_strtoi64(timeout_parameter, , 10);
> > > >   if (errno) {
> > > > @@ -2688,16 +2689,20 @@ AP_DECLARE(apr_status_t) ap_timeout_para
> > > >   time_str = endp;
> > > >   }
> > > >
> > > > +if (tout < 0) {
> > > > +return APR_ERANGE;
> > > > +}
> > > > +
> > > >   switch (*time_str) {
> > > >   /* Time is in seconds */
> > > >   case 's':
> > > >   case 'S':
> > > > -*timeout = (apr_interval_time_t) apr_time_from_sec(tout);
> > > > +check = apr_time_from_sec(tout);
> > > >   break;
> > > >   case 'h':
> > > >   case 'H':
> > > >   /* Time is in hours */
> > > > -*timeout = (apr_interval_time_t) apr_time_from_sec(tout * 
> > > > 3600);
> > > > +check = apr_time_from_sec(tout * 3600);
> > > >   break;
> > > >   case 'm':
> > > >   case 'M':
> > > > @@ -2705,12 +2710,12 @@ AP_DECLARE(apr_status_t) ap_timeout_para
> > > >   /* Time is in milliseconds */
> > > >   case 's':
> > > >   case 'S':
> > > > -*timeout = (apr_interval_time_t) tout * 1000;
> > > > +check = tout * 1000;
> > > >   break;
> > > >   /* Time is in minutes */
> > > >   case 'i':
> > > >   case 'I':
> > > > -*timeout = (apr_interval_time_t) apr_time_from_sec(tout * 
> > > > 60);
> > > > +check = apr_time_from_sec(tout * 60);
> > > >   break;
> > > >   default:
> > > >   return APR_EGENERAL;
> > > > @@ -2719,6 +2724,10 @@ AP_DECLARE(apr_status_t) ap_timeout_para
> > > >   default:
> > > >   return APR_EGENERAL;
> > > >   }
> > > > +if (check > APR_INT64_MAX || check < 0) {
> > >
> > > why "|| check < 0"?
> > > 'check' is unsigned (i.e. apr_uint64_t).
> > >
> >
> > ty, changed in 1892063.
>
>
>
> --
> Eric Covener
> cove...@gmail.com



-- 
Eric Covener
cove...@gmail.com


Re: svn commit: r1892038 - /httpd/httpd/trunk/server/util.c

2021-08-07 Thread Eric Covener
Seems like the fuzzer got farther but is still failing


util.c:2713:26: runtime error: signed integer overflow:
 * 1000 cannot be represented in type 'long'
#0 0x4be247 in ap_timeout_parameter_parse /src/httpd/server/util.c:2713:26

case 'M':
switch (*(++time_str)) {
/* Time is in milliseconds */
case 's':
case 'S':
check = tout * 1000;
break;



Does this require a cast?

On Sat, Aug 7, 2021 at 6:45 AM Eric Covener  wrote:
>
> On Sat, Aug 7, 2021 at 3:00 AM Christophe JAILLET
>  wrote:
> >
> > Hi,
> >
> > Le 06/08/2021 à 15:10, cove...@apache.org a écrit :
> > > Author: covener
> > > Date: Fri Aug  6 13:10:45 2021
> > > New Revision: 1892038
> > >
> > > URL: http://svn.apache.org/viewvc?rev=1892038=rev
> > > Log:
> > > fix int overflow in ap_timeout_parameter_parse
> > >
> > > signed integer overflow in ap_timeout_parameter_parse under fuzzing
> > >
> > > Modified:
> > >  httpd/httpd/trunk/server/util.c
> > >
> > > Modified: httpd/httpd/trunk/server/util.c
> > > URL: 
> > > http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util.c?rev=1892038=1892037=1892038=diff
> > > ==
> > > --- httpd/httpd/trunk/server/util.c (original)
> > > +++ httpd/httpd/trunk/server/util.c Fri Aug  6 13:10:45 2021
> > > @@ -2676,6 +2676,7 @@ AP_DECLARE(apr_status_t) ap_timeout_para
> > >   char *endp;
> > >   const char *time_str;
> > >   apr_int64_t tout;
> > > +apr_uint64_t check;
> > >
> > >   tout = apr_strtoi64(timeout_parameter, , 10);
> > >   if (errno) {
> > > @@ -2688,16 +2689,20 @@ AP_DECLARE(apr_status_t) ap_timeout_para
> > >   time_str = endp;
> > >   }
> > >
> > > +if (tout < 0) {
> > > +return APR_ERANGE;
> > > +}
> > > +
> > >   switch (*time_str) {
> > >   /* Time is in seconds */
> > >   case 's':
> > >   case 'S':
> > > -*timeout = (apr_interval_time_t) apr_time_from_sec(tout);
> > > +check = apr_time_from_sec(tout);
> > >   break;
> > >   case 'h':
> > >   case 'H':
> > >   /* Time is in hours */
> > > -*timeout = (apr_interval_time_t) apr_time_from_sec(tout * 3600);
> > > +check = apr_time_from_sec(tout * 3600);
> > >   break;
> > >   case 'm':
> > >   case 'M':
> > > @@ -2705,12 +2710,12 @@ AP_DECLARE(apr_status_t) ap_timeout_para
> > >   /* Time is in milliseconds */
> > >   case 's':
> > >   case 'S':
> > > -*timeout = (apr_interval_time_t) tout * 1000;
> > > +check = tout * 1000;
> > >   break;
> > >   /* Time is in minutes */
> > >   case 'i':
> > >   case 'I':
> > > -*timeout = (apr_interval_time_t) apr_time_from_sec(tout * 
> > > 60);
> > > +check = apr_time_from_sec(tout * 60);
> > >   break;
> > >   default:
> > >   return APR_EGENERAL;
> > > @@ -2719,6 +2724,10 @@ AP_DECLARE(apr_status_t) ap_timeout_para
> > >   default:
> > >   return APR_EGENERAL;
> > >   }
> > > +if (check > APR_INT64_MAX || check < 0) {
> >
> > why "|| check < 0"?
> > 'check' is unsigned (i.e. apr_uint64_t).
> >
>
> ty, changed in 1892063.



-- 
Eric Covener
cove...@gmail.com


Re: svn commit: r1892038 - /httpd/httpd/trunk/server/util.c

2021-08-07 Thread Eric Covener
On Sat, Aug 7, 2021 at 3:00 AM Christophe JAILLET
 wrote:
>
> Hi,
>
> Le 06/08/2021 à 15:10, cove...@apache.org a écrit :
> > Author: covener
> > Date: Fri Aug  6 13:10:45 2021
> > New Revision: 1892038
> >
> > URL: http://svn.apache.org/viewvc?rev=1892038=rev
> > Log:
> > fix int overflow in ap_timeout_parameter_parse
> >
> > signed integer overflow in ap_timeout_parameter_parse under fuzzing
> >
> > Modified:
> >  httpd/httpd/trunk/server/util.c
> >
> > Modified: httpd/httpd/trunk/server/util.c
> > URL: 
> > http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util.c?rev=1892038=1892037=1892038=diff
> > ==
> > --- httpd/httpd/trunk/server/util.c (original)
> > +++ httpd/httpd/trunk/server/util.c Fri Aug  6 13:10:45 2021
> > @@ -2676,6 +2676,7 @@ AP_DECLARE(apr_status_t) ap_timeout_para
> >   char *endp;
> >   const char *time_str;
> >   apr_int64_t tout;
> > +apr_uint64_t check;
> >
> >   tout = apr_strtoi64(timeout_parameter, , 10);
> >   if (errno) {
> > @@ -2688,16 +2689,20 @@ AP_DECLARE(apr_status_t) ap_timeout_para
> >   time_str = endp;
> >   }
> >
> > +if (tout < 0) {
> > +return APR_ERANGE;
> > +}
> > +
> >   switch (*time_str) {
> >   /* Time is in seconds */
> >   case 's':
> >   case 'S':
> > -*timeout = (apr_interval_time_t) apr_time_from_sec(tout);
> > +check = apr_time_from_sec(tout);
> >   break;
> >   case 'h':
> >   case 'H':
> >   /* Time is in hours */
> > -*timeout = (apr_interval_time_t) apr_time_from_sec(tout * 3600);
> > +check = apr_time_from_sec(tout * 3600);
> >   break;
> >   case 'm':
> >   case 'M':
> > @@ -2705,12 +2710,12 @@ AP_DECLARE(apr_status_t) ap_timeout_para
> >   /* Time is in milliseconds */
> >   case 's':
> >   case 'S':
> > -*timeout = (apr_interval_time_t) tout * 1000;
> > +check = tout * 1000;
> >   break;
> >   /* Time is in minutes */
> >   case 'i':
> >   case 'I':
> > -*timeout = (apr_interval_time_t) apr_time_from_sec(tout * 60);
> > +check = apr_time_from_sec(tout * 60);
> >   break;
> >   default:
> >   return APR_EGENERAL;
> > @@ -2719,6 +2724,10 @@ AP_DECLARE(apr_status_t) ap_timeout_para
> >   default:
> >   return APR_EGENERAL;
> >   }
> > +if (check > APR_INT64_MAX || check < 0) {
>
> why "|| check < 0"?
> 'check' is unsigned (i.e. apr_uint64_t).
>

ty, changed in 1892063.


Re: svn commit: r1892038 - /httpd/httpd/trunk/server/util.c

2021-08-07 Thread Christophe JAILLET

Hi,

Le 06/08/2021 à 15:10, cove...@apache.org a écrit :

Author: covener
Date: Fri Aug  6 13:10:45 2021
New Revision: 1892038

URL: http://svn.apache.org/viewvc?rev=1892038=rev
Log:
fix int overflow in ap_timeout_parameter_parse

signed integer overflow in ap_timeout_parameter_parse under fuzzing

Modified:
 httpd/httpd/trunk/server/util.c

Modified: httpd/httpd/trunk/server/util.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util.c?rev=1892038=1892037=1892038=diff
==
--- httpd/httpd/trunk/server/util.c (original)
+++ httpd/httpd/trunk/server/util.c Fri Aug  6 13:10:45 2021
@@ -2676,6 +2676,7 @@ AP_DECLARE(apr_status_t) ap_timeout_para
  char *endp;
  const char *time_str;
  apr_int64_t tout;
+apr_uint64_t check;
  
  tout = apr_strtoi64(timeout_parameter, , 10);

  if (errno) {
@@ -2688,16 +2689,20 @@ AP_DECLARE(apr_status_t) ap_timeout_para
  time_str = endp;
  }
  
+if (tout < 0) {

+return APR_ERANGE;
+}
+
  switch (*time_str) {
  /* Time is in seconds */
  case 's':
  case 'S':
-*timeout = (apr_interval_time_t) apr_time_from_sec(tout);
+check = apr_time_from_sec(tout);
  break;
  case 'h':
  case 'H':
  /* Time is in hours */
-*timeout = (apr_interval_time_t) apr_time_from_sec(tout * 3600);
+check = apr_time_from_sec(tout * 3600);
  break;
  case 'm':
  case 'M':
@@ -2705,12 +2710,12 @@ AP_DECLARE(apr_status_t) ap_timeout_para
  /* Time is in milliseconds */
  case 's':
  case 'S':
-*timeout = (apr_interval_time_t) tout * 1000;
+check = tout * 1000;
  break;
  /* Time is in minutes */
  case 'i':
  case 'I':
-*timeout = (apr_interval_time_t) apr_time_from_sec(tout * 60);
+check = apr_time_from_sec(tout * 60);
  break;
  default:
  return APR_EGENERAL;
@@ -2719,6 +2724,10 @@ AP_DECLARE(apr_status_t) ap_timeout_para
  default:
  return APR_EGENERAL;
  }
+if (check > APR_INT64_MAX || check < 0) {


why "|| check < 0"?
'check' is unsigned (i.e. apr_uint64_t).

If paranoiac, and want to prevent overflow or wrap around after the 
multiplication, I think that we should compute the maximum acceptable 
value pour each case ('s', 'h', ...) and test against it.


CJ

+return APR_ERANGE;
+}
+*timeout = (apr_interval_time_t) check;
  return APR_SUCCESS;
  }