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);
}