Incorrect restriction of lock-timeout values

Joe Orton joe at manyfish.co.uk
Fri Nov 12 08:57:00 EST 2010


On Sun, Oct 31, 2010 at 09:53:39PM +0100, Werner Baumann wrote:
> In response to a lock discovery request my server ( Apache/2.0.63 (Unix)
> mod_ssl/2.0.63 OpenSSL/0.9.7d DAV/2 Catacomb/static) replied with
> timeout set to "Second-4294936962". This value is less then 2^32 - 1
> and valid according to RFC 4918, 10.7 Timeout Request Header.
> But it is greater than LONG_MAX and therefore Neon aborted the parsing
> and lock discovery failed.
> This patch solves the problem. It does not a strict check but assumes
> any positive value of timeout as valid and only limits it to
> LONG_MAX. But it additionally checks for trailing non numeric
> characters.

Hi Werner,

Is there a server bug here?  That is a timeout of 136 years.

Yes, I don't see any harm in returning invalid timeout values.  I 
presume this fixes the issue for you too:

Index: src/ne_locks.c
===================================================================
--- src/ne_locks.c	(revision 1827)
+++ src/ne_locks.c	(working copy)
@@ -1,6 +1,6 @@
 /* 
    WebDAV Class 2 locking operations
-   Copyright (C) 1999-2006, Joe Orton <joe at manyfish.co.uk>
+   Copyright (C) 1999-2006, 2010, Joe Orton <joe at manyfish.co.uk>
 
    This library is free software; you can redistribute it and/or
    modify it under the terms of the GNU Library General Public
@@ -34,6 +34,7 @@
 #endif
 
 #include <ctype.h> /* for isdigit() */
+#include <errno.h>
 
 #include "ne_alloc.h"
 
@@ -428,9 +429,15 @@
     if (ne_strcasecmp(timeout, "infinite") == 0) {
 	return NE_TIMEOUT_INFINITE;
     } else if (strncasecmp(timeout, "Second-", 7) == 0) {
-	long to = strtol(timeout+7, NULL, 10);
-	if (to == LONG_MIN || to == LONG_MAX)
-	    return NE_TIMEOUT_INVALID;
+        char *p;
+        long to;
+
+        errno = 0;
+        to = strtol(timeout+7, &p, 10);
+        if (errno || *p) {
+            return NE_TIMEOUT_INVALID;
+        }
+
 	return to;
     } else {
 	return NE_TIMEOUT_INVALID;
@@ -482,9 +489,6 @@
     case ELM_timeout:
 	NE_DEBUG(NE_DBG_LOCKS, "Got timeout: %s\n", cdata);
 	l->timeout = parse_timeout(cdata);
-	if (l->timeout == NE_TIMEOUT_INVALID) {
-	    return -1;
-	}
 	break;
     case ELM_owner:
 	l->owner = strdup(cdata);





More information about the neon mailing list