1

I'm learning pointers in C, using Linux. I'm trying to use the strcat function, but it doesn't work and I don't understand why.

I'm passing a username to the main as an argument because I need to concatenate and put a number 1 in the first position of this username. For example if the I got as argument username123 I need to convert this to 1username123

I got this code:

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

int main(int argc, char *arg[]){
    const char *userTemp;
    char *finalUser;

    userTemp = argv[1]; //I got the argument passed from terminal
    finalUser = "1";
    strcat(finalUser, userTemp); //To concatenate userTemp to finalUser
    printf("User: %s\n",finalUser);

    return 0;
}

The code compiles, but I got a segmentation fault error and doesn't know why. Can you please help me going to the right direction?

  • 1
    first argument in `strcat`, `finalUser` in this case, must point the memory address which is mutable and large enough to contain the result. since you cannot mutate the content of memory address where `"1"` is stored, it gets segmentation fault. – ymonad Mar 11 '19 at 06:23

4 Answers4

2

I'm trying to use the strcat function, but it doesn't work and I don't understand why.

For starters, you really shouldn't use strcat(). Use strlcat() instead. The "l" version of this and other functions take an extra parameter that let you tell the function how large the destination buffer is, so that the function can avoid writing past the end of the buffer. strcat() doesn't have that parameter, so it relies on you to make sure the buffer is large enough to contain both strings. This is a common source of security problems in C code. The "l" version also makes sure that the resulting string is null-terminated.

The code compiles, but I got a segmentation fault error and doesn't know why.

Here's the prototype for the function: char *strcat( char *dest, const char *src );

Now, you're calling that essentially like this: strcat("1", someString);. That is, you're trying to append someString to "1", which is a string constant. There's no extra room in "1" for whatever string is in someString, and because you're using a function that will happily write past the end of the destination buffer, your code is effectively writing over whatever happens to be in memory next to that string constant.

To fix the problem, you should:

  1. Switch to strlcat().
  2. Use malloc() or some other means to allocate a destination buffer large enough to hold both strings.
Caleb
  • 124,013
  • 19
  • 183
  • 272
  • `strcat` is *fine* provided you know in advance you have enough space. But at least you didn't attempt to use a fixed size buffer like the other answers :-) – paxdiablo Mar 11 '19 at 06:35
  • @paxdiablo There are a lot of things that are arguably fine if you're careful, but the OP here doesn't yet know what to be careful about, so especially in this context I think consistent use of `strlcat()` is indicated. – Caleb Mar 11 '19 at 06:52
  • 1
    Just keep in mind `strlcat` is not standard. I know OP stated Linux which is presumably why you chose that over the inferior `strncat`, but I prefer to cater for portability, even if it means my code is larger :-). Still, your answer is a good one. – paxdiablo Mar 11 '19 at 06:57
2

It is undefined behaviour in C to attempt to modify a string literal (like "1"). Often, these are stored in non-modifiable memory to allow for certain optimisations.

Let's leave aside for the moment the fact that your entire program can be replaced with:

#include <stdio.h>
int main(int argc, char *argv[]){
    printf("User: 1%s\n", (argc > 1) ? argv[1] : "");
    return 0;
}

The way you ensure you have enough space is to create a buffer big enough to hold whatever you want to do. For example:

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

int main(int argc, char *argv[]){
    // Check args provded.

    if (argc < 2) {
        puts("User: 1");
        return 0;
    }

    // Allocate enough memory ('1' + arg + '\0') and check it worked.

    char *buff = malloc(strlen(argv[1]) + 2); 
    if (buff == NULL) {
        fprintf(stderr, "No memory\n");
        return 1;
    }

    // Place data into memory and print.

    strcpy(buff, "1");
    strcat(buff, argv[1]);
    printf("User: %s\n", buff);

    // Free memory and return.

    free(buff);
    return 0;
}

What you shouldn't do is to allocate a fixed size buffer and blindly copy in the data provided by a user. That's how the vast majority of security problems occur, by people overwriting buffers with unexpected data.

paxdiablo
  • 854,327
  • 234
  • 1,573
  • 1,953
  • *Trying* to write into protected memory is probably the real cause of the segmentation fault, rather than actually doing it and overwriting other data. Good call there. – Caleb Mar 11 '19 at 07:18
-1

You're missing some fundamentals about C.

finalUser = "1";

This is created in "read-only" memory. You cannot mutate this. The first argument of strcat requires memory allocated for mutation, e.g.

char finalUser[32];
finalUser[0] = '1';
paxdiablo
  • 854,327
  • 234
  • 1,573
  • 1,953
Andrew Cheong
  • 29,362
  • 15
  • 90
  • 145
-1

Unlike in other languages there is no real string type in C.

You want this:

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

int main(int argc, char *arg[]){
    const char *userTemp;
    char finalUser[100];   // finalUser can contain at most 99 characters

    userTemp = argv[1]; //I got the argument passed from terminal
    strcpy(finalUser, "1");    // copy "1" into the finalUser buffer
    strcat(finalUser, userTemp); //To concatenate userTemp to finalUser
    printf("User: %s\n",finalUser);

    return 0;
}

or even simpler:

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

int main(int argc, char *arg[]){
    char finalUser[100];   // finalUser can contain at most 99 characters
    strcpy(finalUser, "1");    // copy "1" into the finalUser buffer
    strcat(finalUser, argv[1]); //To concatenate argv[1] to finalUser
    printf("User: %s\n",finalUser);    
    return 0;
}

Disclaimer: for the sake of brevity this code contains a fixed size buffer and no check for buffer overflow is done here.

The chapter dealing with strings in your C text book should cover this.

BTW you also should check if the program is invoked with an argument:

int main(int argc, char *arg[]){
  if (argc != 2)
  {
     printf("you need to provide a command line argument\n");
     return 1;
  }
  ...
Jabberwocky
  • 48,281
  • 17
  • 65
  • 115
  • 1
    You should *never* used fixed-size buffers unless you control what you're putting in there. – paxdiablo Mar 11 '19 at 06:39
  • 2
    You should fix this to use strncpy or strncat (or some other way to prevent buffer overflow). – hyde Mar 11 '19 at 06:42
  • 1
    @paxdiablo disclaimer added. – Jabberwocky Mar 11 '19 at 06:42
  • 2
    @hyde `strncat` is not enough, if the buffer is too small the text is simply truncated which may cause other problems – Jabberwocky Mar 11 '19 at 06:45
  • @Jabberwocky Often true, but in most cases it will prevent UB, and makes it easy to check if buffer was filled (with 1 more than required size buffer this is easy way to detect too long strings). Often whatever API or DB has max size for the string, so using dynamic string in user code is usually not enough either. – hyde Mar 11 '19 at 07:23