Re: How was the majordomo bug found ?

[shipley () merde dis org (Evil Pete)]

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <5296.771271502.1 () merde dis org>

> > From the developper's point of view, using system() or even popen() is a 
> single obvious line of C code, fork()/exec() combination needs about a dozen 
> of lines...
>
> > From the patches from Brent Chapman, it seems that majordomo was using 
> system() or popen()...

no offense Brent but any programmer worth a hill of beans will not use
system()/popen() especially on untrusted  data (eg: look how taintperl
handles such things).

years ago when I was searching for security problems to document
I did a `find /usr/src -name *.c -print | xargs egrep "system|popen`
and found a ton-o-ways.

> There should indeed be a FAQ about how to write 'secure programs'.

I have a document I wrote that talks of this, I will see about digging
it out.

here is a code sample (from the doc)  the I wrote to locate programs
from a path


------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <5296.771271502.2 () merde dis org>
Content-Description: findp.c


#include <stdio.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/param.h>

/* find path to a file */
/* Peter M Shipley 1987 */
char *findp(name)
char *name;
{
char *path, *p;
struct stat statbuf;
static char buffy[MAXPATHLEN +1];

    /*  get path from environment */
    if( (path = getenv("PATH")) == NULL)
        path = "/bin:/usr/bin:/usr/ucb";

    /* copy modifiable copy for strtok */
    path = strcpy(malloc(strlen(path) +1), path);

    /* parse the path */
    for(p = strtok(path, ":"); p != NULL ; p = strtok((char *) NULL, ":")) {

#ifdef SECURE
        /* path must be absolute, not writable by other, owned by root or bin */
        if ( *p != '/'
                || (stat(p, &statbuf) == -1)
                || (statbuf.st_mode & S_IWOTH)
                || (statbuf.st_uid != 0 && statbuf.st_uid != 3))
            continue;
#endif
        /* assemble path */
        (void) strcpy(buffy, (*p == NULL ?  "." : p));
        (void) strcat(buffy, "/");
        (void) strcat(buffy, name);

        /* test for files existance */
        if (stat(buffy, &statbuf) == 0)
            break;
    } /* for each in path */

    return( ( p ? buffy : (char *) NULL) );
}

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <5296.771271502.3 () merde dis org>

While you are at it you might want to add the code:


    for (cpp = environ; cp = *cpp; /* void */) {
            char  **xpp;
            if(strncmp(cp, "LD_", 3) == 0) {
                for (xpp = cpp; xpp[0] = xpp[1]; xpp++);
            } else {
                cpp++;
            }
    }

somewhere before the fork/exec/system/popen is called.  or better yet
delete the entire environment and replace it with a new (static) one.

(note that Pyramid and Apollo systems have special data in there environment
and will have to be special cased so that they are preserved.)


------- =_aaaaaaaaaa0--