Razgrizaces Razgrizaces - 2 months ago 6
C Question

Stack Smashing while using strcpy and strcat

I've been trying to debug this for a while, still can't figure out why this causes a stack smashing error (I think the error code is 6, or abort. Essentially this function takes a directory, opens a file and then puts that file into a function so it can use the file, and then outputs the number of times it goes through the function.

int map(char* dir, void* results, size_t size, int (*act)(FILE* f, void* res, char* fn))
{
printf("%s\n", dir);
char copyDirectory[strlen(dir)+1];
//adds the slash
strcpy(copyDirectory, dir);
strcat(copyDirectory, "/");
//before the psuedocode, get all the files in the directory
int numFiles = nfiles(copyDirectory);
DIR* directory = opendir(copyDirectory);
//if there aren't any files, then we exit
if(numFiles == 0)
{
closedir(directory);
return -1;
}
//reads the file from the directory
struct dirent* readFile = readdir(directory);
int output = 0;
while(readFile!=NULL)
{
if(readFile->d_type==DT_REG)
{
//step 2: obtain filepath
char* fileName = readFile->d_name;
int filePathLength = strlen(dir) + strlen(fileName) + 1;//add one for the slash
char filePath[filePathLength];
memset(filePath, 0, filePathLength); //allocat ememory for file path
strcpy(filePath, strcat(dir, fileName));
//step 3: open file
FILE* file = fopen(filePath, "r");
//if the file is unreachable, exit
if(file==NULL)
{
closedir(directory);
return -1;
}
//step 4: perform some action and store result
strcpy(dir, copyDirectory);
act(file, results, fileName);
//step 5: close file
fclose(file);
//to go through loop: increment the readFile
++output;
}
readFile = readdir(directory);
}
closedir(directory);
return output;
}


Map function with the example.

int map(char* dir, void* results, size_t size, int (*act)(FILE* f, void* res, char* fn))
{
char* copyDirectory = strdup(dir);
DIR* directory = opendir(dir);
int output = 0;
struct dirent* readFile = readdir(directory);
while(readFile!=NULL)
{
if(readFile->d_type==DT_REG)
{
//step 2: obtain filepath
char* fileName = readFile->d_name;
int filePathLength = strlen(dir) + strlen(fileName) +2;//add one for the slash
char filePath[filePathLength+1];
memset(filePath, 0, filePathLength); //allocat ememory for file path
strcpy(filePath, strcat(dir, fileName));
//step 3: open file
FILE* file = fopen(filePath, "r");
//if the file is unreachable, exit
if(file==NULL)
{
closedir(directory);
return -1;
}
//step 4: perform some action and store result
strcpy(dir, copyDirectory);
act(file, results, fileName);
//step 5: close file
fclose(file);
//to go through loop: increment the readFile
++output;
}
readFile = readdir(directory);
}
closedir(directory);
return output;
}
//Sample Map function action: Print file contents to stdout and returns the number bytes in the file.
int cat(FILE* f, void* res, char* filename) {
char c;
int n = 0;
printf("%s\n", filename);
while((c = fgetc(f)) != EOF) {
printf("%c", c);
n++;
}
printf("\n");
return n;
}
int main(int argc, char const *argv[])
{
char directory[]= "../rsrc/ana_light/";
size_t size = 100;
void* results[500];
int mapCat = map(directory, results, size, cat);
printf("The value of map is %d.\n", mapCat);
return EXIT_SUCCESS;
}


Where this fails is after it has executed and it prints to the output. The function should print out the contents of the files you have. The directory list needs to have a "/" at the end. Currently it prints the file contents and exits with the value of how many files it read, but it drops off after it exits with the stack smashing error.

EDIT1: Edited the code to reflect the changes I've made.

EDIT2: Done according to the MCVE standards, I think? Should run if I'm not mistaken.

Answer

First problem: change

    char copyDirectory[strlen(dir)+1];

to

    char copyDirectory[strlen(dir)+2];

Second problem: change

       char filePath[filePathLength];

to

       char filePath[filePathLength+1];

Third problem (change appears to have not been there on first read):

     //strcpy(copyDirectory, dir);
     strcat(copyDirectory, dir);

The commented out code was correct:

    strcpy(copyDirectory, dir);

You've been forgetting spaces for trailing null characters.

Fourth problem: You forgot to handle opendir failing.

Fifth problem: this code is wrong:

        memset(filePath, 0, filePathLength); //allocat ememory for file path
        strcpy(filePath, strcat(dir, fileName));

change to:

         strcpy(filePath, copyDirectory);
         strcat(filePath, fileName);

Don't write to your input variables here. This is a very bad idea.

After undoing the (leaky) strdup for copyDirectory and putting your very innovative local buffer back I was able to get the code to run through to completion.

Comments