Attachment 20020 Details for Bug 54773

[patch] new fix, also covering CAN-2004-396

ccvs-exploit-20040521.diff (text/plain), 4.91 KB, created by Sebastian Krahmer on 2004-05-21 16:40:48 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Sebastian Krahmer

Created: 2004-05-21 16:40:48 UTC

Size: 4.91 KB

patch

obsolete

>--- server.c	Tue Apr  6 16:20:55 2004
>+++ /home/devel/mjc/server.c	Fri May 21 03:53:03 2004
>@@ -1622,8 +1622,7 @@
>     char *cp;
>     char *timefield;
> 
>-    if (error_pending ())
>-	return;
>+    if (error_pending ()) return;
> 
>     if (outside_dir (arg))
> 	return;
>@@ -1637,9 +1636,28 @@
> 	    && strlen (arg) == cp - name
> 	    && strncmp (arg, name, cp - name) == 0)
> 	{
>-	    timefield = strchr (cp + 1, '/') + 1;
>-	    if (*timefield != '=')
>+	    if (!(timefield = strchr (cp + 1, '/')) || *++timefield == '\0')
>+	    {
>+		/* We didn't find the record separator or it is followed by
>+		 * the end of the string, so just exit.
>+		 */
>+		if (alloc_pending (80))
>+		    sprintf (pending_error_text,
>+		             "E Malformed Entry encountered.");
>+		return;
>+	    }
>+	    /* If the time field is not currently empty, then one of
>+	     * serve_modified, serve_is_modified, & serve_unchanged were
>+	     * already called for this file.  We would like to ignore the
>+	     * reinvocation silently or, better yet, exit with an error
>+	     * message, but we just avoid the copy-forward and overwrite the
>+	     * value from the last invocation instead.  See the comment below
>+	     * for more.
>+	     */
>+	    if (*timefield == '/')
> 	    {
>+		/* Copy forward one character.  Space was allocated for this
>+		 * already in serve_entry().  */
> 		cp = timefield + strlen (timefield);
> 		cp[1] = '\0';
> 		while (cp > timefield)
>@@ -1647,8 +1665,17 @@
> 		    *cp = cp[-1];
> 		    --cp;
> 		}
>-		*timefield = '=';
> 	    }
>+	    /* If *TIMEFIELD wasn't "/", we assume that it was because of
>+	     * multiple calls to Is-Modified & Unchanged by the client and
>+	     * just overwrite the value from the last call.  Technically, we
>+	     * should probably either ignore calls after the first or send the
>+	     * client an error, since the client/server protocol specification
>+	     * specifies that only one call to either Is-Modified or Unchanged
>+	     * is allowed, but broken versions of WinCVS & TortoiseCVS rely on
>+	     * this behavior.
>+	     */
>+	    *timefield = '=';
> 	    break;
> 	}
>     }
>@@ -1665,8 +1692,7 @@
>     /* Have we found this file in "entries" yet.  */
>     int found;
> 
>-    if (error_pending ())
>-	return;
>+    if (error_pending ()) return;
> 
>     if (outside_dir (arg))
> 	return;
>@@ -1681,9 +1707,28 @@
> 	    && strlen (arg) == cp - name
> 	    && strncmp (arg, name, cp - name) == 0)
> 	{
>-	    timefield = strchr (cp + 1, '/') + 1;
>-	    if (!(timefield[0] == 'M' && timefield[1] == '/'))
>+	    if (!(timefield = strchr (cp + 1, '/')) || *++timefield == '\0')
> 	    {
>+		/* We didn't find the record separator or it is followed by
>+		 * the end of the string, so just exit.
>+		 */
>+		if (alloc_pending (80))
>+		    sprintf (pending_error_text,
>+		             "E Malformed Entry encountered.");
>+		return;
>+	    }
>+	    /* If the time field is not currently empty, then one of
>+	     * serve_modified, serve_is_modified, & serve_unchanged were
>+	     * already called for this file.  We would like to ignore the
>+	     * reinvocation silently or, better yet, exit with an error
>+	     * message, but we just avoid the copy-forward and overwrite the
>+	     * value from the last invocation instead.  See the comment below
>+	     * for more.
>+	     */
>+	    if (*timefield == '/')
>+	    {
>+		/* Copy forward one character.  Space was allocated for this
>+		 * already in serve_entry().  */
> 		cp = timefield + strlen (timefield);
> 		cp[1] = '\0';
> 		while (cp > timefield)
>@@ -1691,8 +1736,17 @@
> 		    *cp = cp[-1];
> 		    --cp;
> 		}
>-		*timefield = 'M';
> 	    }
>+	    /* If *TIMEFIELD wasn't "/", we assume that it was because of
>+	     * multiple calls to Is-Modified & Unchanged by the client and
>+	     * just overwrite the value from the last call.  Technically, we
>+	     * should probably either ignore calls after the first or send the
>+	     * client an error, since the client/server protocol specification
>+	     * specifies that only one call to either Is-Modified or Unchanged
>+	     * is allowed, but broken versions of WinCVS & TortoiseCVS rely on
>+	     * this behavior.
>+	     */
>+	    *timefield = 'M';
> 	    if (kopt != NULL)
> 	    {
> 		if (alloc_pending (strlen (name) + 80))
>@@ -1747,8 +1801,29 @@
> {
>     struct an_entry *p;
>     char *cp;
>+    int i = 0;
>     if (error_pending()) return;
>-    p = (struct an_entry *) xmalloc (sizeof (struct an_entry));
>+
>+    /* Verify that the entry is well-formed.  This can avoid problems later.
>+     * At the moment we only check that the Entry contains five slashes in
>+     * approximately the correct locations since some of the code makes
>+     * assumptions about this.
>+     */
>+    cp = arg;
>+    if (*cp == 'D') cp++;
>+    while (i++ < 5)
>+    {
>+	if (!cp || *cp != '/')
>+	{
>+	    if (alloc_pending (80))
>+		sprintf (pending_error_text,
>+			 "E protocol error: Malformed Entry");
>+	    return;
>+	}
>+	cp = strchr (cp + 1, '/');
>+    }
>+
>+    p = xmalloc (sizeof (struct an_entry));
>     if (p == NULL)
>     {
> 	pending_error = ENOMEM;

Actions: View | Diff

Attachments on bug 54773: 18913 | 18919 | 18920 | 19603 | 20020 | 20405 | 20524 | 20697