Re: xlock mishandles malformed .signature/.plan

From: Jochen Thomas Bauer (jtbat_private-STUTTGART.DE)
Date: Fri Nov 06 1998 - 08:44:20 PST

  • Next message: J.A. Gutierrez: "Re: another /usr/dt/bin/dtappgather feature!"

    Aaron Campbell wrote:
    
    >I found a bug last night in xlock that I felt should be known.
    
    >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.
    >It's hard to tell how serious this is, but I'm sure it could be harmful in
    >some situations/environments.
    
    I think it is not too serious, as only the following thing will happen:
    (well, at least on an Intel x86)
    
    The local variable "char *home", that will be filled with a pointer to a string
    containing the path to the user's home directory, is declared right after the
    local varibale "char buf[121]", so this pointer will be located right above the
    space left for "char buf[121]" on the stack (remember, the stack grows to
    lower addresses on an Intel x86). This means that if fgets returns NULL and
    therefore c==-1, buf[cr] = '\0' will overwrite the most significant byte
    of the pointer "char *home" with NULL (the least significant byte if you are on a
    big endian machine). However, if you take a closer look at the
    code you will see that
    
    1.)
                                cr = strlen(buf) - 1;
                                    if (buf[cr] == '\n') {
                                            buf[cr] = '\0';
                                    }
    So buf[-1] must have a value of buf[-1]=10 in order to get overwritten by NULL.
    
    2.)
    The pointer "char *home" has at this point already been used to construct the
    full path to the .plan, .signature or .xlocktext file, and the result has already
    been written to a buffer pointed to by "char *buffer".
    
    So, IMHO, there is no security hole created by this bug.
    
    
    Jochen Bauer
    Institute for Theoretical Physics
    University of Stuttgart, Germany
    



    This archive was generated by hypermail 2b30 : Fri Apr 13 2001 - 14:22:20 PDT