strcpy buffer overflow

[setharnold]

openjpeg/src/bin/jp3d/convert.c

Line 201 in 06f7d41

strcpy(pgxfiles[0], relpath);

    char tmpdirpath[MAX_PATH];
    char dirpath[MAX_PATH];
    char pattern[MAX_PATH];
    char pgxfiles[MAX_SLICES][MAX_PATH];
    int pgxslicepos[MAX_SLICES];
    char tmpno[3];

    numcomps = 1;
    color_space = CLRSPC_GRAY;
    sliceno = 0;
    maxvalue = 0;
    memset(pgxfiles, 0, MAX_SLICES * MAX_PATH * sizeof(char));
    memset(&cmptparm, 0, sizeof(opj_volume_cmptparm_t));

    /* Separación del caso de un único slice frente al de muchos */
    if ((tmp = strrchr(relpath, '-')) == NULL) {
        /*fprintf(stdout,"[INFO] A volume of only one slice....\n");*/
        sliceno = 1;
        maxslice = 1;
        strcpy(pgxfiles[0], relpath);   /* XXX possible overflow */

    } else {
        /*Fetch only the path */
        strcpy(tmpdirpath, relpath);    /* XXX possible overflow */
        if ((tmp = strrchr(tmpdirpath, '/')) != NULL) {
            tmp++;
            *tmp = '\0';
            strcpy(dirpath, tmpdirpath);  /* XXX possible overflow */
        } else {
            strcpy(dirpath, "./");           /* XXX possible overflow */
}

Hello, note that the values I found for MAX_PATH (260) and OPJ_PATH_LEN (4096) allow overwriting the smaller buffer with data from the larger buffer. strncpy(3) is difficult enough to use correctly that I suspect a rewrite using sprintf(3) to be the safer approach. Rewriting to avoid the string manipulations might be safer still.

Consider breaking apart the filename and directory entry handling from the image handling in this function. That would make it easier to apply a fuzzer to just the image handling portion, and perhaps also to design test cases that work with the file and directory handling.

Thanks