Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak in pac_utils.h #96

Closed
yoav-lapin opened this issue May 29, 2018 · 3 comments · Fixed by #133
Closed

Memory leak in pac_utils.h #96

yoav-lapin opened this issue May 29, 2018 · 3 comments · Fixed by #133
Milestone

Comments

@yoav-lapin
Copy link

In file pac_utils.h line 339 str_replace.

char *tmporig = malloc(strlen(orig) + 1); // Copy of orig that we work with

The variable tmporig is allocated but never freed or returned, this cause a memory leak in pacparser_find_proxy (and probably some other APIs).

Adding free(tmporig) in the end of the function will solve this leak.
Thank you.

@manugarg manugarg added this to the v1.3.10 milestone Mar 25, 2022
@manugarg manugarg linked a pull request Mar 25, 2022 that will close this issue
@fralken
Copy link

fralken commented Apr 12, 2022

Hello,
the tmporig pointer has moved in this snippet

    while (count--) {
        ins = strstr(tmporig, rep);
        len_front = ins - tmporig;
        tmp = strncpy(tmp, tmporig, len_front) + len_front;
        tmp = strcpy(tmp, with) + len_with;
        tmporig += len_front + len_rep; // move to next "end of rep"
    }

hence the free(tmporig); doesn't work since tmporig doesn't point to the start of the buffer.
Since the buffer pointed by orig parameter is not modified inside the function, there is no need to duplicate it.
So, instead of doing

    char *tmporig = malloc(strlen(orig) + 1); // Copy of orig that we work with
    tmporig = strcpy(tmporig, orig);

it should be enough doing instead

    char *tmporig = orig;

so there is no need to perform a free() in the end, and no memory leak is created.

@manugarg
Copy link
Owner

Good catch, @fralken. My C-foo is pretty rusty now, but we don't really need to even create a copy of orig, right? We could simply use orig; it won't modify the underlying string

@fralken
Copy link

fralken commented Apr 13, 2022

Indeed you are right @manugarg since orig itself is a copy of the pointer to the input string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants