Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 885989 - x11-misc/xscreensaver-6.05-r1 does not make xscreensaver-auth setuid and hence it at risk to OOM killing
Summary: x11-misc/xscreensaver-6.05-r1 does not make xscreensaver-auth setuid and henc...
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: Normal normal
Assignee: Pascal Jäger
URL:
Whiteboard:
Keywords: PullRequest
Depends on:
Blocks:
 
Reported: 2022-12-14 20:09 UTC by Sebastian Pipping
Modified: 2023-04-21 14:32 UTC (History)
1 user (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sebastian Pipping gentoo-dev 2022-12-14 20:09:21 UTC
I came across these lines in ~/.xsession-errors by accident:

>> # grep -F xscreensaver-auth ~/.xsession-errors
>> xscreensaver-auth: 20:50:30: OOM: /proc/22369/oom_score_adj: Permission denied
>> xscreensaver-auth: 20:50:30:   To prevent the kernel from randomly unlocking
>> xscreensaver-auth: 20:50:30:   your screen via the out-of-memory killer,
>> xscreensaver-auth: 20:50:30:   "xscreensaver-auth" must be setuid root.

Inspecting the file shows that there is no setuid bit set indeed:

>> # stat /usr/lib64/misc/xscreensaver/xscreensaver-auth
>>   File: /usr/lib64/misc/xscreensaver/xscreensaver-auth
>>   Size: 305416          Blocks: 600        IO Block: 4096   regular file
>> Device: 253,4   Inode: 804300      Links: 1
>> Access: (0755/-rwxr-xr-x)  Uid: (    0/    root)   Gid: (    0/    root)
>> Access: 2022-10-31 21:03:53.945152323 +0100
>> Modify: 2022-10-31 21:03:53.945152323 +0100
>> Change: 2022-11-03 17:23:21.028740642 +0100
>>  Birth: 2022-11-03 17:23:21.026740626 +0100

Is making the ebuild set the setuid flag the right fix?
Comment 1 Pascal Jäger 2022-12-15 11:14:23 UTC
(In reply to Sebastian Pipping from comment #0)
> Is making the ebuild set the setuid flag the right fix?

We can not do much else. 
xscreensavers build systems actually does this itself. (see configure flag --with-proc-oom, which is always set for us) 
But firstly this setting is not honored by the '/usr/lib/portage/python3.10/ebuild-helpers/xattr/install' program we use when installing (with emake) in an ebuild and secondly the permissions are overwritten during merge. 
I think the only option we have is setting the setuid bit in pkg_postinst().
Comment 2 Pascal Jäger 2022-12-15 12:20:41 UTC
Nevermind what I said permissions being overwritten. It works when putting fperm in src_install(). I don't know what I saw there.
Comment 3 Larry the Git Cow gentoo-dev 2022-12-16 20:19:43 UTC
The bug has been closed via the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=f4662bc9b410044c7f4a2118efb479b1523d591f

commit f4662bc9b410044c7f4a2118efb479b1523d591f
Author:     Pascal Jäger <pascal.jaeger@leimstift.de>
AuthorDate: 2022-12-15 11:25:01 +0000
Commit:     Sebastian Pipping <sping@gentoo.org>
CommitDate: 2022-12-16 20:18:44 +0000

    x11-misc/xscreensaver: revbump, fix bugs
    
    Closes: https://bugs.gentoo.org/885441
    Closes: https://bugs.gentoo.org/885479
    Closes: https://bugs.gentoo.org/885989
    Closes: https://github.com/gentoo/gentoo/pull/28671
    
    Signed-off-by: Pascal Jäger <pascal.jaeger@leimstift.de>
    Signed-off-by: Sebastian Pipping <sping@gentoo.org>

 ...xscreensaver-6.05-r2-configure-exit-codes.patch |  43 ++++
 x11-misc/xscreensaver/xscreensaver-6.05-r2.ebuild  | 248 +++++++++++++++++++++
 2 files changed, 291 insertions(+)
Comment 4 Sebastian Pipping gentoo-dev 2022-12-16 20:21:17 UTC
For the record, I found that configure.ac contains code to conditionally
install the binary with the setuid bit set, bit the script actively decides
against doing that for case USE="pam", see:

>> # If we also have PAM, assume that we don't need to be setuid.
>> if test $have_pam != yes; then
>>   setuid_auth=yes
>> fi

I felt like that should be documented, in case we have to revisit this later.
Comment 5 Karl Hakimian 2023-04-21 13:52:14 UTC
Shouldn't this follow the suid use flag?

That flag is set for this package, but even with it off, xscreensaver-auth is being installed with suid root privileges.
Comment 6 Sebastian Pipping gentoo-dev 2023-04-21 14:32:31 UTC
(In reply to Karl Hakimian from comment #5)
> Shouldn't this follow the suid use flag?
> 
> That flag is set for this package, but even with it off, xscreensaver-auth
> is being installed with suid root privileges.

Without setuid set, either (a) there could be a security issue due to risk of being OOM killed or (b) we do not install the file altogether and then XScreenSaver is unusable altogether.  So personally I think we are good with status quo.  What do you think?