master process handling patch

Patrick Goetz pgoetz at mail.utexas.edu
Thu Jul 22 18:06:22 EDT 2010


On 07/21/2010 05:12 PM, Bron Gondwana wrote:
>
> We decided to pull map_stupidshared.  Are you on the cyrus-devel
> mailing list?
>

Yeah, but I've only been on it for a couple of months -- maybe this was 
discussed previously.

Trying to evaluate C code that consists of lots of little functions with 
no documentation is like playing nethack (you just entered a maze of 
twisty little tunnels....).

For example, one of the debian package maintainers introduced this patch 
to ~/master/master.c:

----------------------------------------------------
@ -195,13 +195,17 @@
      free(a);
  }

-void get_prog(char *path, unsigned size, char *const *cmd)
+void get_prog(char *path, unsigned int size, char *const *cmd)
  {
      if (cmd[0][0] == '/') {
    /* master lacks strlcpy, due to no libcyrus */
    snprintf(path, size, "%s", cmd[0]);
+  path[size-1] = '\0';
+    }
+    else {
+  snprintf(path, size, "%s/%s", SERVICE_PATH, cmd[0]);
+  path[size-1] = '\0';
      }
-    else snprintf(path, size, "%s/%s", SERVICE_PATH, cmd[0]);
  }
----------------------------------------------------

The parameter type correction at the top fixes a bug, but what the code 
null terminating path?  If path is used as a string then this is OK, but 
otherwise it could be overwriting a necessary character.  To properly 
check if this is an OK patch to submit to the bugzilla, I have to track 
down every use of the get_prog function.

>
> They're both being removed in Cyrus 2.4.  GUID is now compulsary,
> so sha1s will be calculated on append.
>

Not sure what "GUID is now compulsory" means, but it turns out I was 
wrong and that sha1 has also been compromised
http://www.schneier.com/blog/archives/2005/02/sha1_broken.html

so hopefully this is just being used to generate checksums and not for 
actually security....



More information about the Cyrus-devel mailing list