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];

        memcpy(wtmpfile, WTMP_FILENAME, 127);
        memcpy(lastlogfile, LASTLOG_FILENAME, 127);

What? This is clearly a buffer overread. memcpy shouldn't be used to copy strings. It's theoretically possible that if WTMP_FILENAME et al were at the end of the text segment, and going past the end of those strings went into non readable memory, the program would SEGV.

Security guys will call this pointless, but as a developer, this is bug that should be fixed.

Popular posts from this blog

Empowering Women in Cybersecurity: InfoSect's 2024 Training Initiative

C++ Memory Corruption (std::string) - part 4

Pointer Compression in V8