OSEC

Neohapsis is currently accepting applications for employment. For more information, please visit our website www.neohapsis.com or email hr@neohapsis.com
 
Subject: Traceroute problems
From: Tim Robbins (fyrebox3n.gumbynet.org)
Date: Sun Aug 27 2000 - 16:42:19 CDT

Hello,

I thought I'd actually do some auditing for once. I had a quick poke around
the code to traceroute (version 1.4a5) and found three obvious problems:

  * The -q option lets you specify the number of `probes'. Specifying a
large
     number here lets you flood the network, I suppose it's similar in a way
     to ping -f. My solution is to limit normal users (uid != 0) to three
     probes, which is the default. There is no limit for the superuser.

  * The optional `packetlen' argument is not sanity-checked. A user can
     specify a huge packet length which will get allocated via malloc(),
     possibly causing a denial-of-service situation. My solution is to limit
     it to the `maxpacket' variable used elsewhere (32768).

  * Will segfault in some situations due to calling free() on memory
     allocated with savestr(). My solution is to use strdup() instead.
         $ traceroute -g 127.0.0.1 -g 127.0.0.1 127.0.0.1
         Segmentation fault

There's probably a few other things I haven't yet spotted, but I really do
think traceroute shouldn't be running setuid root at all. traceroute.c says:

    This program must be run by root or be setuid. (I suggest that
    you *don't* make it setuid -- casual use could result in a lot
    of unnecessary traffic on our poor, congested nets.)

A patch that addresses the three problems I found follows this message. I
doubt if any of them could be used to gain root access, but it's always nice
to have your setuid programs with as few bugs as possible. I hope PINE
didn't chew up the patch.

Cheers,

  Tim

diff -ruN traceroute-1.4a5-old/traceroute.c traceroute-1.4a5/traceroute.c
--- traceroute-1.4a5-old/traceroute.c Fri Jun 13 19:30:27 1997
+++ traceroute-1.4a5/traceroute.c Sun Aug 27 21:29:55 2000
-399,7 +399,7
                        break;

                case 'q':
- nprobes = str2val(optarg, "nprobes", 1, -1);
+ nprobes = str2val(optarg, "nprobes", 1, (getuid() == 0) ? -1 : 3);
                        break;

                case 'r':
-465,7 +465,7

        case 2:
                packlen = str2val(argv[optind + 1],
- "packet length", minpacket, -1);
+ "packet length", minpacket, maxpacket);
                /* Fall through */

        case 1:
-1191,7 +1191,12
        }
        addr = inet_addr(hostname);
        if ((int32_t)addr != -1) {
- hi->name = savestr(hostname);
+ hi->name = strdup(hostname);
+ if (hi->name == NULL) {
+ Fprintf(stderr, "%s: strdup %s\n",
+ prog, strerror(errno));
+ exit (1);
+ }
                hi->n = 1;
                hi->addrs = calloc(1, sizeof(hi->addrs[0]));
                if (hi->addrs == NULL) {
-1212,7 +1217,11
                Fprintf(stderr, "%s: bad host %s\n", prog, hostname);
                exit(1);
        }
- hi->name = savestr(hp->h_name);
+ hi->name = strdup(hp->h_name);
+ if (hi->name == NULL) {
+ Fprintf(stderr, "%s: strdup %s\n", prog, strerror(errno));
+ exit(1);
+ }
        for (n = 0, p = hp->h_addr_list; *p != NULL; ++n, ++p)
                continue;
        hi->n = n;