Search code examples
cstringmemoryoverflow

How to copy a malloced string into another string in C


I have this function that randomly generates a string

char *generate_random_string(int seed) {
    if (seed != 0) {
        srand(seed);
    }
    
    char *alpha_num_str =
                "abcdefghijklmnopqrstuvwxyz"
                "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
                "0123456789";
    
    char *random_str = malloc(RAND_STR_LEN);
    
    for (int i = 0; i < RAND_STR_LEN; i++) {
            random_str[i] = alpha_num_str[rand() % (strlen(alpha_num_str) - 1)];
    }
    
    return random_str;
}

I want to copy the return value of this function if I give it a seed of '1' into a string called initialisation_vector and this is currently how I am doing it:

char initialisation_vector[RAND_STR_LEN + 1] = {0};
strcpy(initialisation_vector, generate_random_string(1));

However, when I run the code I get a malloc buffer overflow error from the strcpy line. What am I doing wrong, how do I allocate enough memory for this?


Solution

  • The main issue:
    Strings is C are expected to be null/zero terminated. This means that an n characters string should be allocated with n+1 bytes and the last one should be '\0'.

    The problem in your code is that random_str is allocated to contain exactly RAND_STR_LEN characters and therefore misses the null termination.

    You should change:

    char *random_str = malloc(RAND_STR_LEN);
    for (int i = 0; i < RAND_STR_LEN; i++) {
        random_str[i] = alpha_num_str[rand() % (strlen(alpha_num_str) - 1)];
    }
    

    To:

    /*-------------------------------------vvv--*/
    char *random_str = malloc(RAND_STR_LEN + 1);
    for (int i = 0; i < RAND_STR_LEN; i++) {
        random_str[i] = alpha_num_str[rand() % (strlen(alpha_num_str) - 1)];
    }
    random_str[RAND_STR_LEN] = '\0';  /* add zero termination */
    

    Note:
    If you want to select random characters from all the available ones in alpha_num_str, you should use: rand() % (strlen(alpha_num_str)) (without the -1). This is because x % n will return values of 0..n-1.
    Also for efficiency reasons you can calculate it once before the loop and store in a variable.