COMMAND

    xlock

SYSTEMS AFFECTED

    Systems running xlock

PROBLEM

    Aaron Campbell  found following.   xlock iteratively  searches for
    .xlocktext, .plan,  and then  a .signature  file in  the invoker's
    home directory, and upon finding  one, opens it for reading.   The
    problem is here, in xlock/xlock.c, under the read_plan() function:

    static void
    read_plan()
    {
            FILE       *planf = NULL;
            char        buf[121];
            char       *home = getenv("HOME");
            char       *buffer;
            int         i, j, cr;
    [...]
                    planf = my_fopen(buffer, "r");
            }
            if (planf != NULL) {
                    for (i = 0; i < TEXTLINES; i++) {
                            if (fgets(buf, 120, planf)) {
                                     cr = strlen(buf) - 1;
    [...]
                                    buf[cr] = '\0';
    [...]

    If  we  generate  a   poisonous  .signature  file,  for   example,
    containing at least one line that begins with a NULL byte, fgets()
    will evaluate to  true but strlen(buf)  will return 0  and cr will
    point outside of buf, obviously bad.

    Thomas  Schweikle  found  two  compilers generating security holes
    this way:

        - Microwerks CodeWarrior for Macintosh
        - ORCA/C for GS/OS (Apple IIgs)
        - various microcontroller compilers

SOLUTION

    The maintainer of  xlockmore has been  notified. Below is  a patch
    from Todd Miller that has already been applied to OpenBSD-current.
    Please save  this then  edit instead  of copy/paste,  a couple  of
    lines are longer than  80 columns.  And  of course, if it  doesn't
    apply, try the -l flag  to patch(1) in case the  whitespace messed
    up somewhere along the way.

    --- xlock.c.orig        Wed Nov  4 20:33:47 1998
    +++ xlock.c     Wed Nov  4 20:34:28 1998
    @@ -2524,7 +2524,7 @@
            char        buf[121];
            char       *home = getenv("HOME");
            char       *buffer;
    -       int         i, j, cr;
    +       int         i, j, len;

            if (!home)
                    home = "";
    @@ -2587,13 +2587,12 @@
            }
            if (planf != NULL) {
                    for (i = 0; i < TEXTLINES; i++) {
    -                       if (fgets(buf, 120, planf)) {
    -                               cr = strlen(buf) - 1;
    -                               if (buf[cr] == '\n') {
    -                                       buf[cr] = '\0';
    +                       if (fgets(buf, 120, planf) && (len = strlen(buf)) > 0) {
    +                               if (buf[len - 1] == '\n') {
    +                                       buf[--len] = '\0';
                                    }
                                    /* this expands tabs to 8 spaces */
    -                               for (j = 0; j < cr; j++) {
    +                               for (j = 0; j < len; j++) {
                                            if (buf[j] == '\t') {
                                                    int         k, tab = 8 - (j % 8);

    @@ -2603,12 +2602,11 @@
                                                    for (k = j; k < j + tab; k++) {
                                                            buf[k] = ' ';
                                                    }
    -                                               cr += tab;
    -                                               if (cr > 120)
    -                                                       cr = 120;
    +                                               len += tab;
    +                                               if (len > 120)
    +                                                       len = 120;
                                            }
                                    }
    -                               buf[cr] = '\0';

                                    plantext[i] = (char *) malloc(strlen(buf) + 1);
                                    (void) strcpy(plantext[i], buf);