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

sca: ignore symlinks when generating so deps #1739

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

imjasonh
Copy link
Member

When processing a package that contains a symlink from, say, /usr/bin/ps to /bin/ps, we shouldn't chase that symlink to scan it for SO deps.

We should still chase .so symlinks that point to other .sos, just not file paths that we scan for ELF dependency metadata.

I'm not confident this is a correct change, so I'd love a really thorough review and ideas about how we should test this.

@smoser
Copy link
Contributor

smoser commented Jan 10, 2025

When processing a package that contains a symlink from, say, /usr/bin/ps to /bin/ps, we shouldn't chase that symlink to scan it for SO deps.

Why not? If it is a dynamic executable in a "normal" path my assumption would be that someone can call it and expect it to work if they've installed the package.

Did you have an example where that is not true?

I guess if the target is also in a normal path, then one could conclude that we would process the target later and thus can ignore the symlink.

@imjasonh
Copy link
Member Author

Did you have an example where that is not true?

We're debugging a case where a -compat package containing a symlink from /usr/bin/ps to /bin/ps picks up ps's SO deps, and it's causing problems. It seemed weird to me that we would chase that symlink.

I guess if the target is also in a normal path, then one could conclude that we would process the target later and thus can ignore the symlink.

Yeah, I think mostly because of this. The symlink itself doesn't have any SO deps, it's just whatever other packages provide the symlink target that need it.

I'm fully willing to be disagreed with here, I don't think I've thought about this as much as you and Jon have.

@smoser
Copy link
Contributor

smoser commented Jan 10, 2025

We're debugging a case where a -compat package containing a symlink from /usr/bin/ps to /bin/ps picks up ps's SO deps, and it's causing problems. It seemed weird to me that we would chase that symlink.

Ah, that makes sense, and I agree with you that the deps don't belong in the compat package. So I guess it depends on the target.

a. need to follow: /usr/bin/myprog -> ../../lib/myprog/bin/myprog
b. don't bother: /usr/bin/ps -> ../bin/ps , where ../bin/ps is in a different package
c. doesn't matter, doesn't hurt: /usr/bin/ps -> ../bin/ps , where ../bin/ps is same package
d. doesn't matter, doesn't hurt: /usr/bin/cat -> busybox (busybox in same package)

I think "pay attention if the destination is owned by this package" works in all those cases. Do you have any others?

(@xnox would point out that usr merge fixes 'b' )

@xnox
Copy link
Contributor

xnox commented Jan 10, 2025

But I thought we flip flopped this, because we do need sca deps on symlinks located at public locations pointing at private libs.

I wonder if this is even worse.

Symlink is absolute. And we parse the build host binary, rather than the built one.

Cause a proper relative symlink, would be likely broken.

If we change this, I want to see a full melange scan --diff.

@xnox
Copy link
Contributor

xnox commented Jan 10, 2025

We shouldn't have and shouldn't process absolute symlinks.

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 this pull request may close these issues.

3 participants