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 overwrite issue for pacparser_find_proxy function #99

Closed
ripplehang opened this issue Oct 8, 2019 · 1 comment · Fixed by #134
Closed

memory overwrite issue for pacparser_find_proxy function #99

ripplehang opened this issue Oct 8, 2019 · 1 comment · Fixed by #134
Milestone

Comments

@ripplehang
Copy link

ripplehang commented Oct 8, 2019

The following piece of code was pacparser_find_proxy

// URL-encode "'" as we use single quotes to stick the URL into a temporary script.
  char ***sanitized_url** = str_replace(url, "'", "%27");
  // Hostname shouldn't have single quotes in them
  if (strchr(host, '\'')) {
    print_error("%s %s\n", error_prefix,
		"Invalid hostname: hostname can't have single quotes.");
    return NULL;
  }

  script = (char*) malloc(32 + strlen(**url**) + strlen(host));
  script[0] = '\0';
  strcat(script, "findProxyForURL('");
  strcat(script, **sanitized_url**);
  strcat(script, "', '");
  strcat(script, host);
  strcat(script, "')");

The above code using stelen(url) to calculate the length of the memory malloced, however, the sanitized_url could be larger than url since one single quote character would be replaced to three characters(%27)
If there are many single quotes in the input url, then it would cause memory overwrite issue.

@manugarg
Copy link
Owner

That's a good observation. Thanks a lot. I'll submit a fix soon.

@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
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.

2 participants