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

pcre2grep should detect symlink recursions #2

Closed
PhilipHazel opened this issue Aug 23, 2021 · 1 comment
Closed

pcre2grep should detect symlink recursions #2

PhilipHazel opened this issue Aug 23, 2021 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@PhilipHazel
Copy link
Collaborator

This is issue 2794 from the old Bugzilla, posted by Thomas Tempelmann. This is the proposed patch:

Assuming this is line 3330:

char buffer[FNBUFSIZ];

Then please rename "buffer" into "childpath" for better readability.

Then insert right after 3352 ("sprintf(..."):

  #if 1 // <-- replace with test for Linux and BSD (macOS/Darwin)
    // prevent endless recursion due to a symlink pointing to a parent dir (Bug 2794)
    char resolvedpath[PATH_MAX];
    if (realpath(childpath, resolvedpath) == NULL)
      continue;     // this path is invalid - we can skip processing this
    BOOL isSame = strcmp(pathname, resolvedpath) == 0;
    if (isSame)
      continue;    // we have a recursion
    strlcat(resolvedpath, "/", sizeof(resolvedpath));
    size_t rlen = strlen(resolvedpath);
    BOOL contained = strncmp(pathname, resolvedpath, rlen) == 0;
    if (contained)
      continue;    // we have a recursion
    resolvedpath[rlen-1] = 0;   // removes the added "/"
    strlcpy(childpath, resolvedpath, sizeof(childpath));
  #endif

I've tested this to work successfully on macOS with my screwed-up symlink.

The tricky part is to tell whether the resolved path is pointing back to where we already were.

With the first strcmp() I check whether it equals the parent current directory path. This assumes, though, that "pathname" is also already resolved - but if the user passed an unresolved path that points to itself, this recursion detection will fail the first time, but then, in the recursion, the paths will be the same (because I replace childpath with the resolved path further down) and thus the recursion will be stopped. You could just as well also resolve the path at the top of the function, but that's wasteful, IMO.

The second strcmp then checks of the resolved path exists as a the current path's parent or their parent. I do this by adding a "/" to the resolved path so that I do not mismatch the case where the resolved is "/a/b" and the parent is "/a/b2".

@PhilipHazel PhilipHazel self-assigned this Aug 23, 2021
@PhilipHazel PhilipHazel added the bug Something isn't working label Aug 23, 2021
@PhilipHazel
Copy link
Collaborator Author

Applied a modified version of the above patch. See ChangeLog for details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant