I took a quick look at the latest NetBSD CGD code and found
out that out of 19 memory allocation operations 11 (almost 60%)
are done in a way that could lead to a segmentation violation
which would leave behind a core dump full of sensitive
information that could be used to compromise a CGD encrypted
disk. While this attack is not very practical since it requires
the attacker to be able to cause resource starvation at a
specific time when cgdconfig is used, it is still possible.
Here are the details...
Memory allocation operations in files in src/sbin/cgdconfig:
params.c 1 wrong, 1 correct
pkcs5_pbkdf2.c 1 wrong, 1 correct
utils.c 9 wrong, 6 correct
params.c:90: p = malloc(sizeof(*p));
params.c-91- params_init(p);
params.c-92- return p;
params.c-93-}
params.c-94-
params.c-95-static void
params.c-96-params_init(struct params *p)
params.c-97-{
params.c-98-
params.c-99- p->algorithm = NULL;
--
pkcs5_pbkdf2.c:103: data = malloc(Slen + 4);
pkcs5_pbkdf2.c-104- memcpy(data, S, Slen);
--
utils.c:94: ret = malloc((nwords+1) * sizeof(char *));
utils.c-95- tmp1 = tmp = strdup(line);
utils.c-96- while ((cur = strsep_getnext(&tmp, " \t")) != NULL)
utils.c-97- ret[i++] = strdup(cur);
--
utils.c:150: out = malloc(sizeof(*out));
utils.c-151- out->length = inlength;
utils.c:152: out->text = malloc(out->length + 1);
utils.c-153- memcpy(out->text, intext, out->length);
utils.c-154- out->text[out->length] = '\0';
--
utils.c:188: sum = malloc(sizeof(*sum));
utils.c-189- sum->length = a1->length + a2->length;
utils.c:190: sum->text = malloc(sum->length + 1);
utils.c-191- memcpy(sum->text, a1->text, a1->length);
--
utils.c-255- /* XXX do some level of error checking here */
utils.c:256: b = malloc(sizeof(*b));
utils.c-257- b->length = len;
utils.c:258: b->text = malloc(BITS2BYTES(b->length));
utils.c-259- memcpy(b->text, buf, BITS2BYTES(b->length));
--
utils.c-323- /* XXX do some level of error checking here */
utils.c:324: b = malloc(sizeof(*b));
utils.c-325- b->length = MAX(x1->length, x2->length);
utils.c:326: b->text = calloc(1, BITS2BYTES(b->length));
utils.c-327- for (i=0; i < BITS2BYTES(MIN(x1->length, x2->length)); i++)
utils.c-328- b->text[i] = x1->text[i] ^ x2->text[i];
Also, using free_notnull() is pointless, that function should be removed
and all calls to that function should be replaced with calls to free(3),
since free(3) takes no action if the pointer is NULL.
utils.c:171: free_notnull(s->text);
utils.c:276: free_notnull(b->text);
utils.c:411: free_notnull(tmp);
utils.c:412: free_notnull(out);
--
utils.c-495-void
utils.c:496:free_notnull(void *b)
utils.c-497-{
utils.c-498-
utils.c-499- if (b)
utils.c-500- free(b);
utils.c-501-}
I find it alarming that this kind of sloppy programming can be found in
a piece of software that is supposedly designed to be secure and provide
security. I believe the CGD code should be seriously audited before
anyone considers using it in a production environment.
ALeine
___________________________________________________________________
WebMail FREE http://mail.austrosearch.net
_______________________________________________
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
