-
Notifications
You must be signed in to change notification settings - Fork 360
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
Non-root owned symlink causes install failure #3100
Comments
I guess this is very much intentional as the owner may send RPM to overwrite random files otherwise. This is the very case we did re-write the fsm for. This still needs to give a sane error message, giving the user a fighting chance to find out what's the issue. |
Yup, we can't follow non-root owned symlinks, the main issue is error message. Ideally we'd toss the bad link with a warning and continue installation but whether that's possible is a whole other question. |
OK, we actually set |
RPM refuses to follow non root owned symlinks pointing to files owned by another user for security reasons. This case was lumped in with other issues resulting in us setting errno to ENOTDIR. This led to confusing as the symlink often indeed points at a directory. Using EPERM is still not 100% right but points at least roughly into the right direction. May be we should catch EPERM further up the call stack and use it to give a more meaningfull error message. Resolves: rpm-software-management#3100
EPERM is just a different kind of misleading: I'm root, what do you mean "not permitted"? |
So seeing this thing was like popping a cork in my head and then all the memories of the symlink CVE's bubbling up throughout yesterday evening to entertain me... Commenting here instead of the PR because there's more to this than what's in the PR now. This error is not a system error so there's no errno that will adequately, much less satisfactorily match the situation. Been there, pored through it. It's an rpm specific error, and that you now added in the PR. I was about to say I don't remember why I didn't add that a specific error for this, but actually I did: 2668a2c. Only I forgot to actually use it 😆 . So, we don't need a new code for it. Don't add any EPERM in there, ENOTDIR is technically the right thing for mimicing O_DIRECTORY behavior as it intends to. Instead, this invalid symlink thing can happen in any number of situations, not all of them related to directories. It needs to be caught inside the "follow links" condition and reported as the already existing RPMERR_INVALID_SYMLINK. And then make sure the O_DIRECTORY logic doesn't mess up the code. I think it doesn't but you'll want to double-check. Finally, besides reporting the invalid symlink as such when encountered, the right thing to do and to actually resolve this ticket is to catch this in the pre-flight checks and prevent the transaction from even starting. It doesn't remove the need to handle it inside the fsm - the fsm needs to catch it real-time like it does now to prevent bad actors, but the pre-flight check should be there to catch admin mistakes such as in the original report. Failing an update mid-transaction can have pretty catastrophic consequences so we should catch what we can. I didn't chase that back then because fixing the CVE was the priority. |
Heh, I'd already forgotten this one: 89ce4e7 - but truly, been there, didn't help 😄 |
Oh, indeed. Actually that's also what the original RHEL ticket is about (i.e. to improve the error reporting here), I just filed this GH ticket in a rush and apparently didn't quite read through the RHEL ticket properly. I'll fix up the "Expected results" here. That's just the "MVP" here, a better (proper) fix would be the one @pmatilai suggested above, of course. |
The MVP to improve user understanding would be using the specific error code we have for this. |
RPM refuses to follow non root owned symlinks pointing to files owned by another user for security reasons. This case was lumped in with other issues resulting in us setting errno to ENOTDIR. This led to confusing as the symlink often indeed points at a directory. Change fsmOpenat to report back an error code and use rpmfilesErrorCodes from rpmarchive.h - which are already used in the surrounding code. Give meaningful error message when encountering unsave symlinks. Retire the abuse of errno for our own reporting. Resolves: rpm-software-management#3100
RPM refuses to follow non root owned symlinks pointing to files owned by another user for security reasons. This case was lumped in with other issues resulting in us setting errno to ENOTDIR. This led to confusing as the symlink often indeed points at a directory. Change fsmOpenat to report back an error code and use rpmfilesErrorCodes from rpmarchive.h - which are already used in the surrounding code. Give meaningful error message when encountering unsave symlinks. Retire the abuse of errno for our own reporting. Resolves: rpm-software-management#3100
RPM refuses to follow non root owned symlinks pointing to files owned by another user for security reasons. This case was lumped in with other issues resulting in us setting errno to ENOTDIR. This led to confusing as the symlink often indeed points at a directory. Change fsmOpenat to report back an error code and use rpmfilesErrorCodes from rpmarchive.h - which are already used in the surrounding code. Give meaningful error message when encountering unsave symlinks. Retire the abuse of errno for our own reporting. Resolves: rpm-software-management#3100
RPM refuses to follow non root owned symlinks pointing to files owned by another user for security reasons. This case was lumped in with O_DIRECTORY behavior, leading to confusing error message as the symlink often indeed points at a directory. Give a more meaningful error message when encountering unsafe symlinks. Co-authored-by: Florian Festi <[email protected]> Resolves: rpm-software-management#3100
RPM refuses to follow non root owned symlinks pointing to files owned by another user for security reasons. This case was lumped in with O_DIRECTORY behavior, leading to confusing error message as the symlink often indeed points at a directory. Emit a more meaningful error message when encountering unsafe symlinks. We already detect the error condition in the main if block here, might as well set the error code right there and then so we don't need to redetect later. We previously only tested for the unsafe link condition when our O_DIRECTORY equivalent was set, but that seems wrong. Probably doesn't matter with the existing callers, but we really must not follow those unsafe symlinks no matter what. Co-authored-by: Florian Festi <[email protected]> Resolves: rpm-software-management#3100
RPM refuses to follow non root owned symlinks pointing to files owned by another user for security reasons. This case was lumped in with O_DIRECTORY behavior, leading to confusing error message as the symlink often indeed points at a directory. Emit a more meaningful error message when encountering unsafe symlinks. We already detect the error condition in the main if block here, might as well set the error code right there and then so we don't need to redetect later. We previously only tested for the unsafe link condition when our O_DIRECTORY equivalent was set, but that seems wrong. Probably doesn't matter with the existing callers, but we really must not follow those unsafe symlinks no matter what. Co-authored-by: Florian Festi <[email protected]> Resolves: rpm-software-management#3100
Describe the bug
Installing a package that ships a file in a directory symlink that's owned by a non-root user results in a cpio unpacking error.
UPDATE:
Failing is expected here of course, the error message can be quite cryptic though.
To Reproduce
Steps to reproduce the behavior:
useradd user
chown -h user /lib
dnf install -y kernel-core
Expected behavior
Better error message.
Output
Environment
Additional context
Filed originally in RH Jira: https://issues.redhat.com/browse/RHEL-33393. The reporter has suggested the culprit in the second comment in that issue.
The text was updated successfully, but these errors were encountered: