A bunch of memory allocation bugs in CGD

From: ALeine (aleine_at_austrosearch.net)
Date: 03/30/05

  • Next message: Miguel Mendez: "Re: A bunch of memory allocation bugs in CGD" 
    Date: Wed, 30 Mar 2005 05:55:17 -0800 (PST)
To: elric@imrryr.org

    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 "freebsd-hackers-unsubscribe@freebsd.org"
  • Next message: Miguel Mendez: "Re: A bunch of memory allocation bugs in CGD"