If the data on the config socket used by spamd-setup is not formatted
correctly we can get a free() of an uninitialized pointer and
potentially a NULL pointer dereference as well.

The problem is that sdl_add() expects the next entry in blacklists[]
to be zeroed out but we only zero out the tag pointer unless we are
reallocating the array.  This can lead to a free() of an uninitialized
pointer if there is a parse error of the config data due to the
sdl_free() call at misc_error.

What's more, for existing entries this can create a "hole" in
blacklists[] which the rest of the code does not expect.  In this
case we need to adjust blu and move entries to fill in the hole.

I also moved the reallocarray() block slightly to make thing a bit
more logical.

Found while modifying the spamd-setup protocol...

 - todd

Index: src/libexec/spamd/sdl.c
===================================================================
RCS file: /cvs/src/libexec/spamd/sdl.c,v
retrieving revision 1.19
diff -u -r1.19 sdl.c
--- src/libexec/spamd/sdl.c     11 Oct 2014 03:25:16 -0000      1.19
+++ src/libexec/spamd/sdl.c     8 Jan 2015 21:11:16 -0000
@@ -73,18 +73,18 @@
        } else {
                if (debug > 0)
                        printf("adding list %s; %d entries\n", sdname, addrc);
-               idx = blu;
-       }
-       if (idx == blu && blu == blc) {
-               struct sdlist *tmp;
+               if (blu == blc) {
+                       struct sdlist *tmp;
 
-               tmp = reallocarray(blacklists, blc + 128,
-                   sizeof(struct sdlist));
-               if (tmp == NULL)
-                       return (-1);
-               blacklists = tmp;
-               blc += 128;
-               sdl_clear(&blacklists[idx]);
+                       tmp = reallocarray(blacklists, blc + 128,
+                           sizeof(struct sdlist));
+                       if (tmp == NULL)
+                               return (-1);
+                       blacklists = tmp;
+                       blc += 128;
+                       sdl_clear(&blacklists[blu]);
+               }
+               idx = blu;
        }
 
        if ((blacklists[idx].tag = strdup(sdname)) == NULL)
@@ -151,7 +151,7 @@
        }
        if (idx == blu) {
                blu++;
-               blacklists[blu].tag = NULL;
+               sdl_clear(&blacklists[blu]);
        }
        return (0);
  parse_error:
@@ -159,6 +159,9 @@
                printf("sdl_add: parse error, \"%s\"\n", addrs[i]);
  misc_error:
        sdl_free(&blacklists[idx]);
+       memmove(&blacklists[idx], &blacklists[idx + 1],
+           (blu - idx) * sizeof(*blacklists));
+       blu--;
        return (-1);
 }
 

Reply via email to