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

modif: lookup xauth in PATH #6087

Merged
merged 1 commit into from
Nov 24, 2023
Merged

Conversation

chestnykh
Copy link
Contributor

Don't use hardcoded /usr/bin/xauth,
iterate over directories inside PATH instead.

This fixes #6006

Copy link
Collaborator

@glitsj16 glitsj16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch!

Copy link
Collaborator

@kmk3 kmk3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see now that there already is a function for doing this: find_in_path() in
fs_lib.c:

Please try using that instead of adding a new one.

@chestnykh
Copy link
Contributor Author

I see now that there already is a function for doing this: find_in_path() in fs_lib.c:

* https://github.com/netblue30/firejail/blob/b689b69f6c3b8a8ba633d6300cef6a19972d53dc/src/firejail/fs_lib.c#L55

Please try using that instead of adding a new one.

done

Copy link
Collaborator

@kmk3 kmk3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks better now, thanks.

Note: There is still a TOCTOU issue with the checks due to this allowing a
potentially user-writable xauth binary, though fixing the issue properly
requires refactoring elsewhere, so I might try that after this PR.

src/firejail/x11.c Outdated Show resolved Hide resolved
Don't use hardcoded `/usr/bin/xauth`,
iterate over directories inside PATH instead.

This fixes netblue30#6006
@netblue30
Copy link
Owner

Merged, thanks!

@netblue30 netblue30 merged commit 5e23f74 into netblue30:master Nov 24, 2023
13 checks passed
@netblue30
Copy link
Owner

Merged, thanks!

@kmk3 kmk3 changed the title Lookup xauth in PATH. modif: Lookup xauth in PATH. Nov 27, 2023
kmk3 added a commit to kmk3/firejail that referenced this pull request Dec 14, 2023
This reverts commit 407c05e.

If --private-lib is used (and firejail is configured with
--enable-private-lib), the following error occurs:

    $ firejail --quiet --noprofile --private-lib true
    firejail: fs_lib.c:56: find_in_path: Assertion `geteuid() != 0' failed.
    Error: proc 10000 cannot sync with peer: unexpected EOF
    Peer 10001 unexpectedly killed (Segmentation fault)

Given that it causes an uid assertion failure, the logic appears to not
be correct and the current behavior may be unsafe, so for now revert
that commit until the issue is properly addressed.

Relates to netblue30#6006 netblue30#6087.

Fixes netblue30#6113.
kmk3 added a commit that referenced this pull request Jan 3, 2024
Reverted by commit 8f33e72 ("Revert "Lookup xauth in PATH."",
2023-12-13) / PR #6129.

Relates to #6006 #6087.
@kmk3 kmk3 changed the title modif: Lookup xauth in PATH. modif: lookup xauth in PATH Sep 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Reverted (RELNOTES N/A)
Development

Successfully merging this pull request may close these issues.

xauth command is hardcoded to /usr/bin/xauth
4 participants