Bug 14600 - hplip creates a world-writeable $HOME/.hplip/hplip.conf file
Summary: hplip creates a world-writeable $HOME/.hplip/hplip.conf file
Status: RESOLVED FIXED
Alias: None
Product: Mageia
Classification: Unclassified
Component: RPM Packages (show other bugs)
Version: Cauldron
Hardware: x86_64 Linux
Priority: High critical
Target Milestone: ---
Assignee: Florian Hubold
QA Contact:
URL:
Whiteboard: 5beta1
Keywords: Security
Depends on:
Blocks:
 
Reported: 2014-11-19 07:56 CET by Juergen Harms
Modified: 2014-11-22 10:27 CET (History)
3 users (show)

See Also:
Source RPM: hplip-3.14.6-5.mga5.src.rpm
CVE:
Status comment:


Attachments

Description Juergen Harms 2014-11-19 07:56:07 CET
Description of problem:

In the list of world-writeable files, msec reports .hplip/hplib.conf for each user. That is a small security problem that should be fixed 


Version-Release number of selected component (if applicable):


How reproducible:
always (fully updated Mageia5 Beta1)

Steps to Reproduce:
1. do ls -la $HOME/.hplip
2.
3.


Reproducible: 

Steps to Reproduce:
Juergen Harms 2014-11-19 07:56:41 CET

Whiteboard: (none) => 5beta1

David Walser 2014-11-19 22:29:03 CET

Assignee: bugsquad => doktor5000

Comment 1 Florian Hubold 2014-11-19 23:56:07 CET
home directories should be 755 by default, and by default users get private groups. Hence I don't see the issue, as other users cannot write to that file.

ââ[user1@localhost]â[23:32:14]â[~]
ââââ¼ umask                                                              
0002
ââ[user1@localhost]â[23:32:16]â[~]
ââââ¼ ls -ald $HOME
drwxr-xr-x 71 user1 user1 4096 Nov 19 23:23 /home/user1/
ââ[user1@localhost]â[23:33:04]â[~]
ââââ¼ ls -al test                                                        
-rw-rw-r-- 1 user1 user1 0 Nov 19 23:33 test
ââ[user1@localhost]â[23:33:07]â[~]
ââââ¼ sudo su - user2                                                    
[user2@localhost ~]$ echo test >> /home/user1/test
-bash: /home/user1/test: permission denied
[user2@localhost ~]$


Although I still agree this is ugly and should be fixed, and according to Debian should has been fixed by them a long time ago: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=293117

Although I can't seem to find the relevant commit ... :/

CC: (none) => doktor5000

Comment 2 Juergen Harms 2014-11-20 10:16:12 CET
In the meantime, I had realised that I was somewhat sloppy in submitting the bug and did not do sufficient testing which would have made me aware of what you point out - sorry.

You are right: the 755 home directory protects a user from having the .hplip/hplip.conf file to be clobbered by another user, even if this is world-writeable. That is not more than ugly, particularly since the problem is on the list of things that will be fixed by debian. I am therefore clsing the bug

Status: NEW => RESOLVED
Resolution: (none) => INVALID

Comment 3 David Walser 2014-11-20 15:08:44 CET
(In reply to Juergen Harms from comment #2)
> You are right: the 755 home directory protects a user from having the
> .hplip/hplip.conf file to be clobbered by another user, even if this is
> world-writeable.

It most certainly does not.

Status: RESOLVED => REOPENED
Resolution: INVALID => (none)

Comment 4 Theodoros Kalamatianos 2014-11-20 15:14:18 CET
Let's clarify something here. A 755 directory permission will _not_ protect any existing world-writable files from being written to by another user. Not unless another directory in the file path has tighter permissions than that.

The missing write bits for the directory mode only mean that other users may not modify the directory itself by creating or removing files. When it comes to the existing files themselves it's the file permissions that come into play.

Therefore, if the file created by hplip is world-writable (e.g. 666) then a 755 mode on its parent directories will not protect it; that is a security issue that needs to be fixed.

That said, the default umask in Mageia is 022, which creates files that are only writable by their owner. If hplip is ignoring the umask in favor of a more permissive mode, then it should be fixed.

And here comes the actual testing:

- On a fresh Mageia 4.1 install, hplip does *not* create a world-writable file.

- On a M5b1-upgraded-to-Cauldron install ~/.hplip/hplip.conf *is* created as world-writable

- On a Cauldron system upgraded from Mageia 4, if I delete the ~/.hplip/ directory and run e.g. `hp-info; the new ~/.hplip/hplip.conf *is* world-writable.

CC: (none) => thkala

Comment 5 Christiaan Welvaart 2014-11-20 15:16:42 CET
For reference, the file gets permissions:
-rw-rw-rw- .hplip/hplip.conf

CC: (none) => cjw

Christiaan Welvaart 2014-11-20 15:17:31 CET

Summary: hplib creates a world-writeable $HOME/.hplip/hplib.conf file => hplib creates a world-writeable $HOME/.hplip/hplip.conf file

Christiaan Welvaart 2014-11-20 15:27:46 CET

Summary: hplib creates a world-writeable $HOME/.hplip/hplip.conf file => hplip creates a world-writeable $HOME/.hplip/hplip.conf file

Thierry Vignaud 2014-11-20 18:05:49 CET

Keywords: (none) => Security
Priority: Normal => release_blocker
Severity: normal => critical

Comment 6 Florian Hubold 2014-11-20 19:00:14 CET
(In reply to Juergen Harms from comment #2)
> particularly since the problem
> is on the list of things that will be fixed by debian. I am therefore clsing
> the bug

Maybe you got me wrong - it is ugly, it should not be world-writeable. And it was fixed _long ago_ by Debian, but seems the fix never made it upstream. And it was quite an issue, excerpt from changelog of the linked debian bug:


Changes: 
 hplip (0.8.7-3) unstable; urgency=low
 .
   * Henrique de Moraes Holschuh:
     * HPLIP:
       * SECURITY FIX: create .hplip.conf on user directory mode 600 (was 666)
         The HPLIP suite was failing to set the process umask to sane values,
         hpssd.py and hpguid.py were affected.  Also, modify HPLIP so that it
         warns the user of the broken permissions, ignores such a file, and
         fixes the permissions on the next time the config file is written to.
         Thanks to Erwan David <erwan@rail.eu.org> for reporting this bug
         (closes: #293117)

If someone could help me track down the commit for that bug that would be greatly appreciated. Debians gitweb and websvn for the packages seems to be unavailable, and patch-tracker.debian.org has been taken offline due to missing maintainer :/

I can fix the umask issue myself probably, but I'd like to have the complete fix instead of having to redo it for nothing ...

Priority: release_blocker => High
Status: REOPENED => ASSIGNED

Comment 7 Christiaan Welvaart 2014-11-20 20:29:51 CET
The python code that causes this problem is shown below, to run, save it e.g. as test.py. Shell commands to show the problem follow.

When the .hplip directory already exists, the new hplip.conf gets the correct permissions. This is also demonstrated below. 

Could a python expert check what is wrong?


rm -rf ~/hpliptest2
python test.py
ls -l ~/hpliptest2
-rw-rw-rw- 1 user user 0 Nov 20 20:21 hplip.conf
rm -f ~/hpliptest2/hplip.conf
python test.py
-rw-r--r-- 1 user user 0 Nov 20 20:25 hplip.conf

=======================8<=============================
import sys
import os
import os.path
import pwd
import stat

def getHPLIPDir():
    homedir = os.path.expanduser('~')
    hplipdir = os.path.join(homedir, "hpliptest2")
    status = 0

    if not os.path.exists(hplipdir):
        try:
            os.umask(0)
            s = os.stat(homedir)
            os.mkdir(hplipdir, 0755)
            os.chown(hplipdir, s[stat.ST_UID], s[stat.ST_GID])
        except OSError:
            status = 1
            log.error("Failed to create %s" % hplipdir)

    return status, hplipdir

sts, user_dir = getHPLIPDir()
user_config_file = os.path.join(user_dir, 'hplip.conf')

if not os.path.exists(user_config_file):
        try:
            file(user_config_file, 'w').close()
            s = os.stat(os.path.dirname(user_config_file))
            os.chown(user_config_file, s[stat.ST_UID], s[stat.ST_GID])
        except IOError:
            pass
Comment 8 Christiaan Welvaart 2014-11-20 20:31:16 CET
Forgot the second "ls -l ~/hpliptest2" but should be clear enough.
Comment 9 Juergen Harms 2014-11-20 20:57:15 CET
Much feedback on a - precipitately - closed bug. Some summarizing:

Bug report + Comments #4 & #5: .hplip/hplip.conf is created world-writeable ( not so on Mageia 4)

Comments #1 and #6: a corresponding bug report exists on Debian, is probably resolved there, but, so far, no correction on Mageia. Florian is pursuing this issue

Commens #2 abd #3: ashes on my head. I just logged in (on a newly installed Mageia5 Beta 1 box) as user guest and had no difficulty to write junk to ~harms/.hplip/hplip.conf). Hence, this is more than uggly and noise in msec reports, it is a real security issue.

The bug is reopened
Comment 10 Christiaan Welvaart 2014-11-20 21:07:31 CET
(In reply to Juergen Harms from comment #9)
> Bug report + Comments #4 & #5: .hplip/hplip.conf is created world-writeable
> ( not so on Mageia 4)

I can confirm, it works correctly in Mageia 4.

> Comments #1 and #6: a corresponding bug report exists on Debian, is probably
> resolved there, but, so far, no correction on Mageia. Florian is pursuing
> this issue

The debian issue is old and was reportedly fixed upstream. So no this is a *new* problem (although it may very well be a recurrence of the same issue).

I verified this permissions problem in debian jessie ("testing" pre-release") and ubuntu utopic (latest release). So no it is not "fixed" there, it is a new problem and their hplip is still broken.

OK now towards a solution; the cause of this mess is the
   umask(0)
in the getHPLIPDir() function. This immediately explains why the permissions are correct if the ~/.hplip directory alrady exists -- getHPLIPDir() is not called in that case so umask is not set to 0.

Now the question is: what is it doing there, can we simply remove this line?
Comment 11 Christiaan Welvaart 2014-11-21 19:09:55 CET
Should be fixed with hplip-3.14.6-7.mga5 .
Comment 12 Juergen Harms 2014-11-22 10:27:44 CET
Is fixed, thanks, closing

Status: ASSIGNED => RESOLVED
Resolution: (none) => FIXED


Note You need to log in before you can comment on or make changes to this bug.