sync_server RESERVE command

David Carter dpc22 at cam.ac.uk
Tue Sep 25 05:53:56 EDT 2007


Two separate bugs fixes, against current 2.3 CVS.

The missing i++ means that we would have failed to RESERVE some messages 
where a given GUID appears multiple times in a mailbox. Not a huge deal.

The missing (i < count) was potentially serious: we fall off the end of 
the ids array where RESERVE doesn't involve the last message in a mailbox.

I think that we got away with it. My reasoning follows:

If you are really unlucky 12/20 bytes of allocated but uninitiated memory 
(ids[count]) will match one of the GUIDs later in the mailbox. This will 
cause cmd_reserve() to reserve that message and issue a spurious response.

Fortunately sync_client will moan and then ignore the response if it 
wasn't expecting the GUID. If it _was_ expecting the GUID then sync_server 
has reserved a legitimate copy. In fact this should be impossible. If 
sync_client wanted a message with that GUID, it would have asked for that 
copy in that mailbox, so the ids array would have been longer.

ids[count] is always allocated.

There is a one in hundred chance that ids[count+1] is not, which would 
lead to a segmentation fault if ids[count] matched a GUID.

-- 
David Carter                             Email: David.Carter at ucs.cam.ac.uk
University Computing Service,            Phone: (01223) 334502
New Museums Site, Pembroke Street,       Fax:   (01223) 334679
Cambridge UK. CB2 3QH.

Index: imap/sync_server.c
===================================================================
RCS file: /cvs/src/cyrus/imap/sync_server.c,v
retrieving revision 1.11
diff -u -d -r1.11 sync_server.c
--- imap/sync_server.c	24 Sep 2007 12:48:32 -0000	1.11
+++ imap/sync_server.c	25 Sep 2007 09:28:09 -0000
@@ -1465,14 +1465,16 @@
          goto cleanup;
      }

-    for (i = 0, msgno = 1 ; msgno <= m.exists; msgno++) {
+    for (i = 0, msgno = 1 ; (msgno <= m.exists) && (i < count); msgno++) {
          mailbox_read_index_record(&m, msgno, &record);

          if (!message_guid_compare(&record.guid, &ids[i]))
              continue;

-        if (sync_message_find(message_list, &record.guid))
+        if (sync_message_find(message_list, &record.guid)) {
+            i++;
              continue; /* Duplicate GUID on RESERVE list */
+        }

          /* Attempt to reserve this message */
          snprintf(mailbox_msg_path, sizeof(mailbox_msg_path),


More information about the Cyrus-devel mailing list