segmentation fault in my code with strcpy

Question

I am getting segmentation fault after strcpy(buffer_two, argv[1]); Not sure what's going wrong here and i would appreciate a hand to understand what's wrong and why i get that segmentation fault.

#include <stdio.h>
#include <string.h>

int main(int argc, char *argv[])
{
    int value = 5;
    char buffer_one[8], buffer_two[8];

    strcpy(buffer_one, "one"); // Put "one" into buffer_one
    strcpy(buffer_two, "two"); // Put "two" into buffer_two
    printf("[BEFORE] buffer_two is at %p and contains \'%s\'\n", buffer_two, buffer_two);
    printf("[BEFORE] buffer_one is at %p and contains \'%s\'\n", buffer_one, buffer_one);

    printf("\n[STRCPY] copying %d bytes into buffer_two\n\n", strlen(argv[1])); //Copy first argument into buffer_two

    strcpy(buffer_two, argv[1]); // <---- here

    printf("[After] buffer_two is at %p and contains \'%s\'\n", buffer_two, buffer_two);
    printf("[After] buffer_one is at %p and contains \'%s\'\n", buffer_one, buffer_one);
    printf("[AFTER] value is at %p and is %d (0x%08x)\n", &value, value, value);     
}

What did you pass as the first argument to the program (i.e. what is `argv[1]`)? — Kninnug, May 31 '18 at 19:06
You should check `argc` before assuming something is in `argv[index]`. It also would not hurt to use `strncpy` to limit the copy to the length of the buffer, since the program has no control over how long `argv[1]` is and could cause a buffer overflow. — Christian Gibbons, May 31 '18 at 19:14
Aside: `buffer_one, buffer_one` ==> `(void*)buffer_one, buffer_one` when supplying `%p` specifier. — Weather Vane, May 31 '18 at 19:22
The reason for the failure is largely dependent on how you call this program. What is the exact command you give to run it? — dbush, May 31 '18 at 19:36

Support Ukraine · Answer 1 · 2018-05-31T20:11:49.410

There are two possible reasons for the seg fault.

1) There is no argv[1] , i.e. you try to copy from a NULL pointer (That is... If the program is started with no arguments, argv[1] may be accessed but will return a NULL pointer. Consequently, copying from it is illegal and may cause a seg fault). So if you start the program like ./program the program will crash as argv[1] is NULL

2) The length of argv[1] exceeds the destination, i.e. 7 chars and a terminating NUL. If so you write out of bounds and may cause a seg fault.

So to get the code correct do:

int main(int argv,char * argv[])
{
    char buffer_two[8];
    if ((argc >  1) && (strlen(argv[1]) < 8)) // Make sure arg[1] is there
                                              // Make sure it's not too long
    {
            strcpy(buffer_two, argv[1]);
    }
    else
    {
        printf("Illegal start of program\n");
    }
    return 0;
}

BTW

When printing a pointer using %p make sure to cast to void*

So this

printf("[After] buffer_two is at %p and contains \'%s\'\n", buffer_two, buffer_two);

should be

printf("[After] buffer_two is at %p and contains \'%s\'\n", (void*)buffer_two, buffer_two);

@WeatherVane Thanks... Of cause... Maybe I'm too tired and should call it the day. Thanks for correcting me :-) — Support Ukraine, May 31 '18 at 19:58

score 2 · Answer 2 · answered May 31 '18 at 20:02

There are two primary problems here: you do not check argc to see if argv[1] exists, and you blindly copy argv[1] into the buffer that might not be big enough to fit it. I've also fixed some of the compiler warnings related to your printf statements. Here is an example, using your code, of how to handle such things:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main(int argc, char *argv[])
{
   if(argc < 2) {
      printf("Please enter an argument when invoking this program\n");
      return EXIT_FAILURE;
   }
    int value = 5;
    char buffer_one[8], buffer_two[8];

    strcpy(buffer_one, "one"); // Put "one" into buffer_one
    strcpy(buffer_two, "two"); // Put "two" into buffer_two
    printf("[BEFORE] buffer_two is at %p and contains \'%s\'\n", (void *)buffer_two, buffer_two);
    printf("[BEFORE] buffer_one is at %p and contains \'%s\'\n", (void *)buffer_one, buffer_one);

    printf("\n[STRCPY] copying %zu bytes into buffer_two\n\n", strlen(argv[1])); //Copy first argument into buffer_two

    strncpy(buffer_two, argv[1], 8); // <---- here
    if(buffer_two[7] != '\0'){
       printf("[ERROR] string did not fit into buffer, truncating\n");
       buffer_two[7] = '\0';
    }

    printf("[After] buffer_two is at %p and contains \'%s\'\n", (void *)buffer_two, buffer_two);
    printf("[After] buffer_one is at %p and contains \'%s\'\n", (void *)buffer_one, buffer_one);
    printf("[AFTER] value is at %p and is %d (0x%08x)\n", (void *)&value, value, value);     
}

Sadaananth Anbucheliyan · Answer 3 · 2018-05-31T19:52:25.977

0

Check argc value before using argv. Use strcpy only when argc >1 . Make a conditional statement before strcpy()

Eg...

int main(int argv,char * argv[])
{
    char buffer[10];
    if (argc >  1 && strlen(argv[1]) < 10)
    {
        strcpy(&buffer[0],argv[1]);
    }
    return 0;
}

You should run program by

./a.out hai

hai is the argument passed. Here argc is 2 ,argv[1] = hai

edited May 31 '18 at 19:52

answered May 31 '18 at 19:08

Sadaananth Anbucheliyan

424
1
5
15

segmentation fault in my code with strcpy

3 Answers3