Posts

Showing posts from March, 2018

axmail

In axmail

char username[20];
char fullname[31];
...
        /* Strip SSID */
        if (local) {
                pw = getpwuid(getuid());
        } else {
                strcpy(callsign, call);
                strcpy(username, callsign);
                strlwr(username);
                p = strchr(username, '-');
                if (p) *p = '\0';
                pw = getpwnam(username);
        }
...
                if (local) {
                        strcpy(username, pw->pw_name);
                        strcpy(callsign, username);
                }
                /* Strip full name from the gecos field... */
                if (strchr(pw->pw_gecos, ',') == NULL)
                        strcpy(fullname, pw->pw_gecos);
                else
                        strcpy(fullname, strtok(pw->pw_gecos, ","));

This seems to be a common legacy code bug - assumptions about username/gecos lengths etc.

Forensics Bugs (#2 rifitui)

rifitui is a tool to recover Windows recycle bins.

  int currrecoff;
  int recordsize;

...

  pread( info2_file, fourbytes, 4, 0x0C );
  recordsize = bah_to_i( fourbytes, 4 );

  record = malloc( recordsize );

...

  while (eof == 0) {
    res = pread( info2_file, record, recordsize, currrecoff );
    if (res < recordsize) {
      eof = 1;
    } else {
      filename = record + 0x04;
      index = bah_to_i( record+0x108, 4 );       drive = bah_to_i( record+0x10C, 4 );
      deltime = win_time_to_unix( record+0x110 );       deltm = localtime( &deltime );       year = deltm->tm_year + 1900;       mon = deltm->tm_mon + 1;       sprintf( ascdeltime, "%02d/%02d/%02d %02d:%02d:%02d", mon, deltm->tm_mday, year, deltm->tm_hour, deltm->tm_min, deltm->tm_sec );
      filesize = bah_to_i( record+0x118, 4 );
      printf( "%d%s%s%s%d%s%s%s%d\n", index, delim, ascdeltime, delim, drive, delim, filename, delim, filesize );     }     currrecoff = currrecoff + recordsize;
It's no…

Forensics Bugs (#1 recoverjpeg)

recoverjpeg

    const char *buffer = file_name(dir_format, file_format, begin_index + i);
      i++;
      if (verbose) {
        printf("%s %ld bytes\n", buffer, (long) size);
      }
      fdout = open(buffer, O_WRONLY | O_CREAT, 0666);
      if (fdout < 0) {
        fprintf(stderr, "Unable to open %s for writing\n", buffer);
        exit(1);
      }

...

Writes to an output file (e.g., image00000.jpg) and doesn't check for it being a symlink - hence an attacker could create a symlink pointing to a privileged file that the person running recoverjpeg has write access to. This is mitigated in /tmp /var/tmp by the Linux kernel, but it's still a bug.

unhide (part #3)

Stack overflow in unhide. There is a mismatch between the maxpathlen from a readlink (it gets both sizes wrong in any case).

An attack scenario might be that a rootkit is installed by an attacker then gets code execution (again presumably) when the sysadmin tries to "unhide" the rootkit.

char cmdcont[1000] ;
...
     char proc_exe[512] ;
...
      sprintf(mypath,"/proc/%d",my_pid);
      statuscmd = stat(mypath, &buffer) ;
      if ((statuscmd == 0) && S_ISDIR(buffer.st_mode))
      {
         pid_exists[N_PROC] = TRUE ;
         strcat(mypath,"/exe") ;
         length = readlink(mypath, cmdcont, 1000) ;
         if (-1 != length)
         {
            cmdcont[length] = 0;   // terminate the string
//            printf("cmdcont(proc_exe) = %s\n", cmdcont) ;   //DEBUG
            strcpy(proc_exe,cmdcont) ;
         }
         else
         {
            strcpy(proc_exe,"unknown exe") ;
         }
      }

unhide (part #2)

In the unhide source we have the following lines of code

// sysctl kernel.pid_max
int maxpid = 32768;

Lets look at what my Linux system that has unhide says.
# sysctl kernel.pid_max kernel.pid_max = 131072
More code in unhide:
unsigned int proc_parent_pids[65536] ;
char *proc_tasks[65536]; char *ps_pids[65536]; char *messages_pids[65536]; char message[1000] ; char description[1000] ; int ps_count = 0 ;
This looks like it's assuming 16bit PIDs.. I won't investigate further, but it's likely to be a problem that will allow for a rootkit to bypass unhide.

unhide (part #1)

In the Linux package unhide, which is rootkit detection software.

// Temporary string for output
char scratch[1000] ;
char cmdcont[1000] ;

...
               size_t length ;                char myexe[512] ;
               sprintf(myexe,"%s%s/exe",mypath,directory); //               printf("%s\n",myexe);
               length = readlink(myexe, cmdcont, 1000) ;
This use of readlink() is in a few places in the code. The trouble is that PATH_MAX isn't 1000.
# getconf -a|grep PATH_MAX PATH_MAX                           4096 _POSIX_PATH_MAX                    4096
This probably leads to a rootkit bypass for unhide.

chkrootkit (part #4)

In chkrootkit

#define MAX_ID 99999

int main(int argc, char*argv[]) {
        int             fh_wtmp;
        int             fh_lastlog;
        struct lastlog  lastlog_ent;
        struct utmp     utmp_ent;
        long            userid[MAX_ID];

...
        for (i=0; i<MAX_ID; i++)                 userid[i]=FALSE;
...
                if (*uid > MAX_ID)                 {                    fprintf(stderr, "MAX_ID is %ld and current uid is %ld, please check\n\r", (long int)MAX_ID, (long int)*uid );                    exit (1);
                }
uid gets set by getpwnam(). On modern systems, it can be 32bits. Much higher than MAX_ID of 99999. If your backdoored account has a high uid, it won't be detected in lastlog/wtmp rootkit detection.
This is not unusually bad of chkrootkit. It's just old code that hasn't been maintained.

chkrootkit (part #3)

In chkrootkit


    if (!quiet)
    {
      signal(SIGALRM, read_status);
      alarm(5);
    }

...

void read_status() {
   double remaining_time;
   static long last_total_bytes_read=0;
   int diff;

   diff = total_wtmp_bytes_read-last_total_bytes_read;
   if (diff == 0) diff = 1;
   remaining_time=(wtmp_file_size-total_wtmp_bytes_read)*5/(diff);
   last_total_bytes_read=total_wtmp_bytes_read;

   printf("Remaining time: %6.2f seconds\n", remaining_time);
/*
   signal(SIGALRM,read_status);

   alarm(5);
*/
}

I'll just quote the man page for signal()

DESCRIPTION
       The behavior of signal() varies across UNIX versions, and has also var‐
       ied historically across different versions of Linux.   Avoid  its  use:
       use sigaction(2) instead.  See Portability below.

Is it a security bug? Unlikely. Is it a bug? Maybe. Should it be fixed? Yes, if you want to maintain it..

chkrootkit (part #2)

Subsequently to the previous bug http://blog.infosectcbr.com.au/2018/03/chkrootkit.html

int main(int argc, char*argv[]) {
        int             fh_wtmp;
        int             fh_lastlog;
        struct lastlog  lastlog_ent;
        struct utmp     utmp_ent;
        long            userid[MAX_ID];
        long            i, slot;
        int             status = 0;
        long            wtmp_bytes_read;
        struct stat     wtmp_stat;
        struct s_localpwd       *localpwd;
        uid_t           *uid;
    int         quiet = 0;

        char wtmpfile[128], lastlogfile[128];

        memcpy(wtmpfile, WTMP_FILENAME, 127);
        memcpy(lastlogfile, LASTLOG_FILENAME, 127);
        while (--argc && ++argv) /* poor man getopt */         {            if (!memcmp("-f", *argv, 2))            {               if (!--argc)                  break;               ++argv;               memcpy(wtmpfile, *argv, 127);            }            else if (!memcmp("-l", *argv, 2))            {   …

chkrootkit (part #1)

#ifdef __FreeBSD__ 
#define WTMP_FILENAME "/var/log/wtmp"
#define LASTLOG_FILENAME "/var/log/lastlog"
#endif
#ifdef __OpenBSD__
#include <stdlib.h> 
#define WTMP_FILENAME "/var/log/wtmp"
#define LASTLOG_FILENAME "/var/log/lastlog"
#endif
#ifndef WTMP_FILENAME
#define WTMP_FILENAME "/var/log/wtmp"
#endif
#ifndef LASTLOG_FILENAME
#define LASTLOG_FILENAME "/var/log/lastlog"
#endif

So WTMP_FILENAME and LASTLOG_FILENAME are pretty bog standard C strings.
int main(int argc, char*argv[]) {         int             fh_wtmp;         int             fh_lastlog;         struct lastlog  lastlog_ent;         struct utmp     utmp_ent;         long            userid[MAX_ID];         long            i, slot;         int             status = 0;         long            wtmp_bytes_read;         struct stat     wtmp_stat;         struct s_localpwd       *localpwd;         uid_t           *uid;     int         quiet = 0;
        char wtmpfile[128], lastlogfile[128];
   …

Wireshark (#2)

In the packet-usbip.c dissector

            num_of_devs = tvb_get_ntohl(tvb, offset);
            offset += 4;

            if (num_of_devs == 0)
                return expected_size;

            if (tvb_captured_length_remaining(tvb, offset) < (gint) (0x138 * num_of_devs))
                return 0;

            for (i = 0; i < num_of_devs; i++) {
                guint8 num_of_intf = tvb_get_guint8(tvb, offset + 0x137);
                int skip = num_of_intf * 4;

                expected_size += 0x138 + skip;
                offset += 0x138 + skip;
            }
            return expected_size;

Integer overflow with 0x138 * num_of_devs.

Does it lead to memory corruption? I'm not sure. Perhaps, perhaps not. I'm really more looking for the presence of input validation bugs as opposed to what they can affect.

2 bugs do make a right in Wireshark

Lets look at the current Wireshark source.

table_length =  tvb_get_ntohl(tvb, offset);

Looks like it's grabbing a 32bit integer.

tf = proto_tree_add_item(info_tree, hf_address_table_length, tvb, offset, 4, ENC_BIG_ENDIAN);
  element_tree = proto_item_add_subtree(tf, ett_table_element);
  EAT(4);

And it uses up 4 bytes with that EAT(4).

if (wccp_wccp_address_table->in_use == FALSE) {
    wccp_wccp_address_table->family = family;
    wccp_wccp_address_table->table_length =  table_length;

    /* check if the length is valid and allocate the tables if needed */
    switch (wccp_wccp_address_table->family) {
    case 1:
      if (wccp_wccp_address_table->table_ipv4 == NULL)
        wccp_wccp_address_table->table_ipv4 = (guint32 *)
          wmem_alloc0(pinfo->pool, wccp_wccp_address_table->table_length * 4);

We have an integer overflow in the alloc on 32 bit systems (where size_t is 32bits). These are potentially useful since you can get a heap overflow if you have an all…