InfoSect's Month of Pointless Bonus Bugs (#33)

InfoSect, Canberra's hackerspace, regularly runs public group sessions to perform code review and vulnerability discovery. Over the next 30 days, I'll highlight the source code of 30 unknown vulnerabilities.

Bug #33

In bchunk, a program to do conversion between CD's bin/cue and iso/cdr.

/*
 *      Convert a mins:secs:frames format to plain frames
 */

long time2frames(char *s)
{
        int mins = 0, secs = 0, frames = 0;
        char *p, *t;
        
        if (!(p = strchr(s, ':')))
                return -1;
        *p = '\0';
        mins = atoi(s);
        
        p++;
        if (!(t = strchr(p, ':')))
                return -1;
        *t = '\0';
        secs = atoi(p);
        
        t++;
        frames = atoi(t);
        
        return 75 * (mins * 60 + secs) + frames;
}


A negative return value is clearly not correct, but there is no sign check.

                        printf(" %s %s", p, t);
                        track->startsect = time2frames(t);
                        track->start = track->startsect * SECTLEN;
...

        printf("Writing tracks:\n\n");
        for (track = tracks; (track); track = track->next)
                writetrack(binf, track, basefile);
                
...

int writetrack(FILE *bf, struct track_t *track, char *bname)
{
        char *fname;
        FILE *f;
        char buf[SECTLEN+10];
        long sz, sect, realsz, reallen;
        char c, *p, *p2, *ep;
        int32_t l;
        int16_t i;
        float fl;
        
...
        if (fseek(bf, track->start, SEEK_SET)) {
                fprintf(stderr, " Could not fseek to track location: %s\n", strerror(errno));
                exit(4);
        }
        
        reallen = (track->stopsect - track->startsect + 1) * track->bsize;

Is it a useful bug to a security researcher? Can the above be misused with a negative startsect variable? My goal isn't to verify or trigger the bug in this particular case. It's just to identify unsafe code that is clearly buggy and that should be fixed. At this point, are we doing dev work, or security work?

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