CODEWITHSUNDEEP
cpass-by-referencedynamic-memory-allocationfunction-definitionaddress-sanitizer
choco02
choco02

Reputation: 31

Heap use after free that i don't understand

I know from previous C projects that ASAN tells me "heap-use-after-free" when I try to access to a pointer after the use offree() but in this case I don't even understand because i use free() at the end of the main and do nothing after.

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

#define URL "URL"
#define API_KEY "KEY"

void add_parameter(char *query, char *key, char *value)
{
    const size_t new_len = strlen(query) + strlen(key) + strlen(value) + 3; 
    query = realloc(query, new_len * sizeof(char));
    if (query)
    {
        strcat(query, "?");
        strcat(query, key);
        strcat(query, "=");
        strcat(query, value);
        query[new_len - 1] = '\0';
        printf("%s\n", query);
    }
}

int main()
{
    char *query = malloc((strlen(URL) + 1) * sizeof(char));
    memcpy(query, URL, sizeof(URL));

    add_parameter(query, "api_key", API_KEY);
    printf("%s\n", query);
    free(query);

    return 0;
}

Thanks for your help

Upvotes: 1

Views: 898

Answers (2)

Vlad from Moscow
Vlad from Moscow

Reputation: 311058

For starters the pointer query is passed to the function add_parameter by value

add_parameter(query, "api_key", API_KEY);

It means that the function deals with a copy of the value of the pointer query within the function.

Changing the copy does not reflect on the original pointer query. The original pointer query passed to the function by value stays unchanged.

You need to pass the pointer by reference.

Passing by reference in C means passing an object indirectly through a pointer to it. Dereferencing the pointer you will have a direct access to the original object.

In this case the function will be declared like

int add_parameter(char **query, const char *key, const char *value);

(as the strings pointed to by the parameters key and value are not changed within the function the parameters should be declared with the qualifier const) and within the function you have to write

int add_parameter( char **query, const char *key, const char *value )
{
    const size_t new_len = strlen( query ) + strlen( key ) + strlen( value ) + 3; 
    char *tmp = realloc( query, new_len * sizeof( char ) ); 

    int success = tmp != NULL;

    if ( success )
    {
        strcat( rmp, "?" );
        strcat( tmp, key );
        strcat( tmp, "=" );
        strcat( tmp, value );
        printf("%s\n", tmp);
   
        *query = tmp;
    }

    return success;
}

This statement within the function

tmp[new_len - 1] = '\0';

is redundant. So it is removed

Pay attention to that realloc can return a null-pointer. In this case if not to use the intermediate variable tmp the early allocated memory can be lost.

The function can be called like

add_parameter( &query, "api_key", API_KEY);

or

if ( add_parameter( &query, "api_key", API_KEY) )
{
    //...
}
else
{
    //...
}

Upvotes: 4

Ed Heal
Ed Heal

Reputation: 60017

The following line can cause a problem

query = realloc(query, new_len * sizeof(char));

As the value of query could change. It, main. it is unaware of this change. Pass the updated version of query back to main.

Upvotes: 3

Related Questions