CODEWITHSUNDEEP
cstack-smash
John Novosel
John Novosel

Reputation: 1

execv() system call creating stacking smashing error

Whenever I used the execv() here in my code, it works and has no errors, but still causes stack smashing to crash the program during runtime. Am I doing anything wrong here? Here is the function with the execv():

void execute(char *args[], char *cmd[])
{
    pid_t pid;
    int status;
    char bin[10] = "/bin/";
    pid = fork();

    // child process
    if(pid == 0)
    {
         strcat(bin, cmd[0]);

    execv(bin, args);   
    } else{
   perror("error");
    while(wait(&status) != pid);
    }
}

here is where I am getting args and cmd from. Would it possibly be caused by something I did here?

void parseString(char *command)
{
    char **args = malloc(sizeof(char*) * 16);
    int i = 0;
    int j = 0;
    char *cmd[1];

    // split each command by semicolons if necessary, then send each sub        command to parseString()
    if(strchr(command, ';')) {
        char *semi_token = strtok(command, ";");
        while(semi_token != NULL){
            args[i] = semi_token;
            semi_token = strtok(NULL, " ");
            parseString(args[i]);
            i++;
        }
    } else {
        // if no semi colons, split the commandby spaces and call execute() using the args and cmd
        char *token = strtok(command, " ");
        while(token != NULL)
        {
            args[i] = token;
            args[++i] = NULL;
            while(j == 0 && token != NULL) {
                    cmd[0] = token;
                    cmd[1] = NULL;
                    j++;
                }
            token = strtok(NULL, " ");
            }
            execute(args, cmd);
        }

        j = 0;
        i = 0;
        free(args);
    }

function call happens here. command is input from stdin from the user. Only need basic commands all located in /bin/. something like ls -l or cat file. 

while(1){
        command = getCommand();
        parseString(command);
}

Upvotes: 0

Views: 225

Answers (1)

Some programmer dude
Some programmer dude

Reputation: 409166

You have two serious errors: One that will lead to out-of-bounds writing of an array, and one that will probably lead to that.

The first, the certain out-of-bounds writing, is in the parseString function. First you have the declaration of the cmd variable:

char *cmd[1];

This defines cmd as an array of one element. Then you do

cmd[0] = token;
cmd[1] = NULL;

which writes to two elements of the one-element array. Writing out of bounds leads to undefined behavior.

The second error is in the execute function, and is the one I talked about in my first comment. There you have

char bin[10] = "/bin/";

That defines bin as an array of ten characters, and you fill up six of them (don't forget the string terminator). In the child-process you do

strcat(bin, cmd[0]);

which appends to string in cmd[0] to the string in bin. The problem here is that bin only have space for ten characters, of which six is already used (as explained above). That means there's only space left for four characters. If the command is any longer than that you will also go out of bounds and again have undefined behavior.

The solution to the first error is simply, make cmd an array of two elements. The solution to the second error is to either make bin larger, and not concatenate more than can fit in the array; Or to allocate the array dynamically (not forgetting space for the terminator).

There are also lot of other potential problems with your code, like the limit on 16 pointers for args. And that you don't really parse arguments in the parseString function, every argument is seen as a separate command. And the memory leak in the case there are semicolon-separated "commands". Or that you don't check for or handle errors everywhere needed. And that you use errno even if there's no error.

Upvotes: 4

Related Questions