TOCTTOU - Using access before handling file

264 Views Asked by At

I have this function that copies files to a mirror directory and then deletes the old one. It works well however, in Coverity, my code shows up as a TOCTTOU warning.

void function(){
    const char *original_key = "path/to/file/filename";
    const char *mirror_key = "path/to/another/file/filename";

    if((access(original_key, F_OK) == 0) && (access(mirror_key, F_OK) == 0)){
        copy_file("/bin/cp %s %s", original_key, mirror_key); /*copy function in another cpp file*/
        unlink(original_key);
    }
}

/* in another cpp file */
int copy_file(const char*command, ...){
    int rc = -1;
    va_list args;
    char *buffer = NULL;
    va_start(args, command);
    vasprintf(&buffer, command, args);
    va_end(args);
    if (buffer)
    {
        char *wrd;
        char *ptr = buffer;
        std::vector<const char *> list;
        while ((wrd = strsep(&ptr, " ")) != NULL)
        {
            if (strlen(wrd))
            {
                list.push_back(wrd);
            }
        }
        if (list.size() > 0)
        {
            char *argv[list.size() + 1];
            for (size_t idx = 0; idx < list.size(); idx++)
            {
                argv[idx] = (char *)list[idx];
            }
            argv[list.size()] = NULL;
            rc = system_spawn_args(argv);
        }
        free(buffer);
    }
    return(rc);
}

Is there a way to prevent TOCTTOU in this situation?

note: move did not work - Cross Device Error.

Thanks

2

There are 2 best solutions below

0
JezT On BEST ANSWER

Since my main function was basically to move one file to another. I just used rename() this did the job and also did not warn me as a TOCTTOU in Coverity

0
A M On

The problem is maybe a typical Unix / Linux bug using the function access. The exact details, even with an example, are described here. There are also recomendations on how to mitigate the risk.

Since it is described very well in the linked Wikepedia article, I will not repeat it here.

And, because there is no C++ code in your example, you may want to rewrite the code in C++. Also your "copy-function" using a system call, seems to be overly complex.

Please consider to refactor it. Therefor I would also recomend to you to read an article about copying a file here on stackoverflow.

If your code is non productive and for home use only, then you can also ignore this message.