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

symlinks within a scanned directory tree are parsed outside the tree, failing if target does not exist #1860

Open
deitch opened this issue Jun 5, 2023 · 5 comments · Fixed by #1861
Labels
bug Something isn't working

Comments

@deitch
Copy link
Contributor

deitch commented Jun 5, 2023

What happened:

Here is a simple test case:

$ mkdir /tmp/syft
$ touch /tmp/syft/a /tmp/syft/b
$ ln -s /foo/bar /tmp/syft/link

The link can point to anywhere, as long as it both starts with / and does not exist.

The error you get will be:

2023/06/01 13:43:01 error during command execution: 1 error occurred:
        * unable to determine resolver while cataloging packages: unable to create directory resolver: unable to index filesystem path="/foo/bar": lstat /foo: no such file or directory

What you expected to happen:

Ideally, it would recognize that /tmp/syft/link points outside of the scanned directory and not follow it. At minimal, it should handle it and not fail.

Steps to reproduce the issue:

See above.

Anything else we need to know?:

The commit that caused the issue is 6afbffce

Root cause analysis

Coming in an edit

@deitch deitch added the bug Something isn't working label Jun 5, 2023
@deitch
Copy link
Contributor Author

deitch commented Jun 5, 2023

Root cause analysis

When it walks through the directory tree in directory_indexer.go, and specifically indexTree, it sees the symlink and parses the target here in indexPath().

It then recognizes that this is a new root (target starts with /), and adds it to the roots, which are returned to the caller indexAllRoots().

It then tries to index this new root, in the above example /foo/bar, bu calling indexTree() again.

Prior to the above commit, it would try to walk the new root, see nothing, and just keep going.

The above commit adds this section which checks of the root is a real path. Overall, that is a good thing for indexTree(), but in this case, it cannot stat /foo and then just fails, returning the error.

Recommendations

It is my opinion that we should not be following symlinks at all. Either way, it either hinders or doesn't help:

  • if the target of the link is inside the directory we are parsing, then we will get to it anyways
  • if the target of the directory is outside the directory we are parsing, then it should be treated as out of scope and ignored

However, if we still desire to have directory parsing, then we should ignore symlinks whose target cannot be found.

A somewhat better alternative (more correctly, addition) would be to have an option either to ignore symlinks (thus fitting with my proposed logic above) or follow them.

Either way, I think "this symlink points nowhere" is a good reason to skip it. The fix for that is fairly simple as well.

When we parse the symlink, here, check if the target exists. If not, do not return the target.

	if _, err := os.Stat(targetAbsPath); err != nil && errors.Is(err, os.ErrNotExist) {
            targetAbsPath = ""
        }
	return targetAbsPath, nil

@deitch
Copy link
Contributor Author

deitch commented Jun 5, 2023

Here is a branch off of latest main with that patch. Works wonderfully https://github.com/deitch/syft/tree/handle-outside-symlinks

If you want, I can open a PR from it.

@jpalermo
Copy link
Contributor

We're running v0.83.1 which includes the fix but we're getting the errors still:

* unable to determine resolver while cataloging packages: unable to create directory resolver: unable to index filesystem path="/tmp/tmp.283DGJmbvR/dev/fd/1": unable to index branch="/tmp/tmp.283DGJmbvR/dev/fd/1": lstat /proc/694/fd/pipe:[801872502]: no such file or directory

The errors exist when running with v0.83.0 and v0.83.1 but do not exist on v0.82.0

In our case, we're mounting an image and scanning it, so the /dev/fd/1 symlink is pointing outside of the mount point to somewhere under /.

CC @selzoc

@spiffcs spiffcs added this to OSS Jul 19, 2023
@spiffcs spiffcs moved this to Backlog in OSS Jul 19, 2023
@spiffcs
Copy link
Contributor

spiffcs commented Jul 19, 2023

Reopened because of a new report from @jpalermo - @anchore/tools

@spiffcs spiffcs reopened this Jul 19, 2023
@jpalermo
Copy link
Contributor

For anybody else running into this problem. We've worked around it by scanning the mounted image using chroot to keep the symlinks correct.

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
Status: Backlog
Development

Successfully merging a pull request may close this issue.

3 participants