Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!

Bug 603408 (CVE-2017-14484)

Summary: <sci-mathematics/gimps-28.10-r1: root privilege escalation in init script
Product: Gentoo Security Reporter: Michael Orlitzky <mjo>
Component: AuditingAssignee: Gentoo Security Audit Team <security-audit>
Status: RESOLVED FIXED    
Severity: major CC: paolo.pedroni, sci-mathematics, tomka
Priority: High    
Version: unspecified   
Hardware: All   
OS: Linux   
See Also: https://bugs.gentoo.org/show_bug.cgi?id=540006
Whiteboard: C1 [glsa cve]
Package list:
Runtime testing required: ---
Attachments: 0001-sci-mathematics-gimps-Fix-potential-privilege-escala.patch
0002-sci-mathematics-gimps-revbump-for-new-version.patch
0003-sci-mathematics-gimps-Improve-documentation-for-firs.patch

Description Michael Orlitzky gentoo-dev 2016-12-22 01:31:45 UTC
The init script for gimps calls "chown -R" on /var/lib/gimps every time you start it:

  checkconfig() {
      ...
      /bin/chown -R ${USER}:${GROUP} ${GIMPS_DIR}

The GIMPS user can place a hard link in that directory to any path he chooses (on the same filesystem), and trick root into giving ownership of that file to him. For example, on my machine, GIMPS is running as nobody:nobody. If I replace /var/lib/gimps/local.txt with a hardlink to /var/lib/portage/world and then start gimps, the "nobody" user gains control of it:

  $ sudo su nobody -c 'ln /var/lib/portage/world /var/lib/gimps/local.txt'
  $ sudo /etc/init.d/gimps start
   * Starting GIMPS ...
  $ ls /var/lib/portage/world
  -rw-r--r-- 2 nobody nobody 3.7K 2016-12-21 16:10 /var/lib/portage/world

Our gentoo-sources have a patch that prevents this, but vanilla-sources do not.
Comment 1 Thomas Kahle (RETIRED) gentoo-dev 2016-12-22 05:56:45 UTC
I'm adding Paolo Pedroni as I'm sort of a proxy maintainer here.  
Why is that chown in the init script in the first place?  Can't we fix the permissions once in the ebuild?
Comment 2 Thomas Kahle (RETIRED) gentoo-dev 2016-12-22 05:57:25 UTC
I'm also going into devaway for a week in 5 minutes.  Security, please feel free to implement any proposed solution.
Comment 3 Paolo Pedroni 2016-12-22 06:28:15 UTC
(In reply to Thomas Kahle from comment #1)
> I'm adding Paolo Pedroni as I'm sort of a proxy maintainer here.  
> Why is that chown in the init script in the first place?  Can't we fix the
> permissions once in the ebuild?

Beats me. As far as I know it's always been there. I've always thought it was there to allow the user under which we choose to run the program to view and edit files in the folder. If there's a better way, please go ahead and fix it.
Comment 4 Kristian Fiskerstrand gentoo-dev Security 2016-12-22 10:15:39 UTC
Adding bug 540006 to see-also as it is referenced in report re attack mitigation (It was here the protect in gentoo-sources was added) as well as changes done to checkpath to mitigate this. As such one alternative is likely to switch to using checkpath in the init script for this package as well.
Comment 5 Paolo Pedroni 2016-12-22 10:45:03 UTC
(In reply to Kristian Fiskerstrand from comment #4)
> Adding bug 540006 to see-also as it is referenced in report re attack
> mitigation (It was here the protect in gentoo-sources was added) as well as
> changes done to checkpath to mitigate this. As such one alternative is
> likely to switch to using checkpath in the init script for this package as
> well.

So something like:
--- gimps.old   2016-12-22 11:40:26.132295240 +0100
+++ gimps.new   2016-12-22 11:42:27.402909801 +0100
@@ -12,7 +12,7 @@
                /bin/mkdir "${GIMPS_DIR}"
        fi
 
-       /bin/chown -R ${USER}:${GROUP} ${GIMPS_DIR}
+       checkpath -d -o ${USER}:${GROUP} ${GIMPS_DIR}
 
        if [ ! -e "${GIMPS_DIR}/local.txt" ]; then
                eerror "GIMPS has not been configured.  Please configure it manually before"

should work, shouldn't it?

I can't test it, though, because I use systemd.
Comment 6 Paolo Pedroni 2016-12-22 10:54:04 UTC
Or even better, to check files in the directory as well:
--- gimps.old   2016-12-22 11:40:26.132295240 +0100
+++ gimps.new   2016-12-22 11:42:27.402909801 +0100
@@ -12,7 +12,7 @@
                /bin/mkdir "${GIMPS_DIR}"
        fi
 
-       /bin/chown -R ${USER}:${GROUP} ${GIMPS_DIR}
+       checkpath -d -o ${USER}:${GROUP} ${GIMPS_DIR}
+       checkpath -f -o ${USER}:${GROUP} ${GIMPS_DIR}/*
 
        if [ ! -e "${GIMPS_DIR}/local.txt" ]; then
                eerror "GIMPS has not been configured.  Please configure it manually before"

if checkpath allows wildcards...
Comment 7 Michael Orlitzky gentoo-dev 2016-12-22 12:54:09 UTC
Checkpath will create the directory if it doesn't exist, so once you've added the

  checkpath -d -o ${USER}:${GROUP} ${GIMPS_DIR}

line, this becomes redundant:

  if [ ! -e "${GIMPS_DIR}" ]; then
      einfo "Creating ${GIMPS_DIR}"
      /bin/mkdir "${GIMPS_DIR}"
  fi

It looks like,

  * Checkpath will accept wildcards, and
  * Checkpath won't operate on a hardlink.

So while this line,

  checkpath -f -o ${USER}:${GROUP} ${GIMPS_DIR}/*

is probably safe, I have to wonder if it's necessary. Once the GIMPS user/group owns $GIMPS_DIR, the only people who can create files in it are root and the GIMPS user. If root creates a file in there, then you don't want to change its ownership, and if the GIMPS user does, it will already have the correct ownership.

I think we could get away with the single call to "checkpath -d"... now all we need is someone who actually runs GIMPS on OpenRC to try it =)
Comment 8 Michael Orlitzky gentoo-dev 2016-12-22 13:09:47 UTC
(In reply to Michael Orlitzky from comment #7)
> 
> So while this line,
> 
>   checkpath -f -o ${USER}:${GROUP} ${GIMPS_DIR}/*
> 
> is probably safe,

Since that wildcard will catch directories, too, the init script may spit out some errors complaining that you're calling "checkpath -f" on a directory. It will also happily create a file named "*" when the directory is empty...
Comment 9 Paolo Pedroni 2016-12-22 13:15:32 UTC
(In reply to Michael Orlitzky from comment #8)
> (In reply to Michael Orlitzky from comment #7)
> > 
> > So while this line,
> > 
> >   checkpath -f -o ${USER}:${GROUP} ${GIMPS_DIR}/*
> > 
> > is probably safe,
> 
> Since that wildcard will catch directories, too, the init script may spit
> out some errors complaining that you're calling "checkpath -f" on a
> directory. It will also happily create a file named "*" when the directory
> is empty...

Ok, scratch that, then.

Since tomka is apparently on devaway, will you accept a patch (or a github PR) with all other proposed changes from me?
Comment 10 Michael Orlitzky gentoo-dev 2016-12-22 13:23:15 UTC
Yeah, sure. I run OpenRC, so if you can tell me the usual steps that you'd take to configure GIMPS, I can try it out.
Comment 11 Paolo Pedroni 2016-12-23 13:31:55 UTC
Created attachment 457286 [details, diff]
0001-sci-mathematics-gimps-Fix-potential-privilege-escala.patch

This is a patch with the intended fix.

To check:
1) emerge the modified package (doh!)
2) Before starting the service run (so it will create some files in /var/lib/gimps, among which /var/lib/gimps/local.txt, whose existance is checked in the init script):
   # /opt/gimps/mprime -m -w/var/lib/gimps
3) Answer 'N' to the "Join Gimps?" question
4) Press enter a few times to accept default values until the stress test starts
5) Stop the stress test with ^C
6) Exit the client, pressing '5'
7) The files in /var/lib/gimps now have the uid and gid of the user under which you run the stress test. Chown them to nobody:nobody or change /etc/conf.d/gimps accordingly (see why the init script happily 'chown'ed the folder and all its contents?)
8) Try to run the init script
Comment 12 Michael Orlitzky gentoo-dev 2016-12-23 14:26:34 UTC
(In reply to Paolo Pedroni from comment #11)
> Created attachment 457286 [details, diff] [details, diff]
> 0001-sci-mathematics-gimps-Fix-potential-privilege-escala.patch
> 
> This is a patch with the intended fix.
> 

Thanks! Can you also do a revision bump of the 28.10 version?

The way this usually goes is that the security team will fast-track the stabilization of your new (fixed) revision, and then we can remove v28.9 from the tree. The new stable revision will force everyone to upgrade, ensuring that they pull down the fixed init script.

(We're not supposed to modify stable ebuilds, but it's kind of a moot point if we're going to get rid of it ASAP.)
Comment 13 Paolo Pedroni 2016-12-23 15:05:58 UTC
Created attachment 457290 [details, diff]
0002-sci-mathematics-gimps-revbump-for-new-version.patch

Revbump for new version.
Comment 14 Michael Orlitzky gentoo-dev 2016-12-23 16:11:52 UTC
Thanks again, I just pushed those two commits so that we can get the stabilization process rolling.

I don't like adding another step (the chown) to the manual configuration, but some of it seems unavoidable. Am I correct that users have to run mprime at least once to generate their local.txt?

If so, it might make sense to document all of that in a special gentoo README file (a la readme.gentoo-r1.eclass) or on the wiki (which could be mentioned in an einfo message).

Here's my totally untested idea.

1. Always run the daemon as a new user, "gimps". That's safer than reusing "nobody", and changing $USER/$GROUP in the conf.d file doesn't work anyway, because the file ownership is wrong after you do it.

2. Create /var/lib/gimps as gimps:gimps in the ebuild.

3. After installation, instruct users what to do, if they want to run the daemon:
 
   3a. Give the "gimps" user a shell temporarily.

   3b. Run "/opt/gimps/mprime -m -w/var/lib/gimps" once **as the gimps user**
       to generate local.txt, and use Ctrl-C to kill it.

   3c. Make /sbin/nologin the shell for "gimps" again, and lock the
       account with "passwd -l".

4. You can remove the call to checkpath in the init script, and use "gimps" instead of $USER/$GROUP. I guess $GIMPS_DIR should be fixed at /var/lib/gimps, too.

That way you never have to run mprime as root, and root never tries to change ownership or permissions on anything. Users still have to do some manual steps, but everything except the shell juggling and account locking is already necessary.
Comment 15 Paolo Pedroni 2017-01-04 12:26:42 UTC
(In reply to Michael Orlitzky from comment #14)
> Thanks again, I just pushed those two commits so that we can get the
> stabilization process rolling.

Sorry for the late reply, but I was on holiday with nearly no internet access.

> 
> I don't like adding another step (the chown) to the manual configuration,
> but some of it seems unavoidable. Am I correct that users have to run mprime
> at least once to generate their local.txt?

Either that, or importing an old local.txt from another installation.

> 
> If so, it might make sense to document all of that in a special gentoo
> README file (a la readme.gentoo-r1.eclass) or on the wiki (which could be
> mentioned in an einfo message).

I will work on it (if tomka does not beat me to it).

> 
> Here's my totally untested idea.
> 
> 1. Always run the daemon as a new user, "gimps". That's safer than reusing
> "nobody", and changing $USER/$GROUP in the conf.d file doesn't work anyway,
> because the file ownership is wrong after you do it.
> 
> 2. Create /var/lib/gimps as gimps:gimps in the ebuild.
> 
> 3. After installation, instruct users what to do, if they want to run the
> daemon:
>  
>    3a. Give the "gimps" user a shell temporarily.
> 
>    3b. Run "/opt/gimps/mprime -m -w/var/lib/gimps" once **as the gimps user**
>        to generate local.txt, and use Ctrl-C to kill it.
> 
>    3c. Make /sbin/nologin the shell for "gimps" again, and lock the
>        account with "passwd -l".
> 
> 4. You can remove the call to checkpath in the init script, and use "gimps"
> instead of $USER/$GROUP. I guess $GIMPS_DIR should be fixed at
> /var/lib/gimps, too.

I'd rather not remove the option of changing user:group for gimps, there are use cases for which it may be necessary. I especially don't like to remove the option of changing $GIMPS_DIR: dual boot users need to change that to put gimps file in a folder readable by Windows.

Of course the final decision lies with tomka as the official mantainer.
Comment 16 Thomas Kahle (RETIRED) gentoo-dev 2017-01-04 14:05:39 UTC
(In reply to Paolo Pedroni from comment #15)
> I will work on it (if tomka does not beat me to it).

I'm in the process of retiring as a dev.

gimps should be maintained by the sci-mathematics project in the future.  Good luck.
Comment 17 Paolo Pedroni 2017-01-04 14:05:40 UTC
Created attachment 458730 [details, diff]
0003-sci-mathematics-gimps-Improve-documentation-for-firs.patch

(In reply to Michael Orlitzky from comment #14)
> If so, it might make sense to document all of that in a special gentoo
> README file (a la readme.gentoo-r1.eclass) or on the wiki (which could be
> mentioned in an einfo message).

This patch adds a README.gentoo file with instructions to first-time users and links to further upstream documentation (without revbump, since we're still unstable).
Comment 18 Paolo Pedroni 2017-01-04 14:08:31 UTC
(In reply to Thomas Kahle from comment #16)
> (In reply to Paolo Pedroni from comment #15)
> > I will work on it (if tomka does not beat me to it).
> 
> I'm in the process of retiring as a dev.
> 
> gimps should be maintained by the sci-mathematics project in the future. 
> Good luck.

:-(((

It's been nice working with you, tomka. Good luck with your future, then.

If the sci-mathematics project agrees I am available to be a proxy-mantainer for this package.
Comment 19 Michael Orlitzky gentoo-dev 2017-01-09 17:50:37 UTC
@Paolo: I've added you as a proxy maintainer in metadata.xml, I'm sure the sci-math project will appreciate the help. I also merged the latest patch with two minor fixes: I killed some trailing whitespace, and added an "|| die" to a "cp" call in the ebuild.

@Security: This is done, we can proceed with stabilization on amd64 and x86, target =sci-mathematics/gimps-28.10-r1.
Comment 20 Paolo Pedroni 2017-04-05 10:26:25 UTC
(In reply to Michael Orlitzky from comment #19)
> @Paolo: I've added you as a proxy maintainer in metadata.xml, I'm sure the
> sci-math project will appreciate the help. I also merged the latest patch
> with two minor fixes: I killed some trailing whitespace, and added an "||
> die" to a "cp" call in the ebuild.

Thanks.

> @Security: This is done, we can proceed with stabilization on amd64 and x86,
> target =sci-mathematics/gimps-28.10-r1.

=sci-mathematics/gimps-28.10-r1 is stable on all archs (see bug #612926).

Old versions should be removed. Can someone in sci-mathematics@g.o do it, please? Or should I push a PR on github?
Comment 21 Michael Orlitzky gentoo-dev 2017-04-06 23:56:49 UTC
(In reply to Paolo Pedroni from comment #20)
> 
> Old versions should be removed. Can someone in sci-mathematics@g.o do it,
> please? Or should I push a PR on github?

I took care of it, thanks again.
Comment 22 Paolo Pedroni 2017-04-07 08:00:32 UTC
(In reply to Michael Orlitzky from comment #21)
> I took care of it, thanks again.

Thanks to you. Security can close this, then?
Comment 23 Michael Orlitzky gentoo-dev 2017-08-17 19:29:49 UTC
Security team: the vulnerable versions of this were removed back in April, what's next?
Comment 24 Kristian Fiskerstrand gentoo-dev Security 2017-08-18 10:09:01 UTC
(In reply to Michael Orlitzky from comment #23)
> Security team: the vulnerable versions of this were removed back in April,
> what's next?

As issue is fixed in stable I'm removing the group restriction for this issue.

We need to request a CVE and produce a GLSA containing said CVE.

@maintainer: In which version was the affected init script added. I take it this is a Gentoo-specific issue and the init script is not included upstream?
Comment 25 Paolo Pedroni 2017-08-18 13:57:16 UTC
(In reply to Kristian Fiskerstrand from comment #24)
> @maintainer: In which version was the affected init script added. I take it
> this is a Gentoo-specific issue and the init script is not included upstream?

The init script was added before I was the mantainer. You are correct this is a Gentoo-specific issue.
Comment 26 Yury German Gentoo Infrastructure gentoo-dev Security 2017-09-10 06:40:34 UTC
New GLSA Request filed.
Comment 27 Christopher Díaz Riveros (RETIRED) gentoo-dev Security 2017-09-15 01:40:00 UTC
CVE requested. I'll update the alias as soon as it is confirmed.

Gentoo Security Padawan
ChrisADR
Comment 28 Christopher Díaz Riveros (RETIRED) gentoo-dev Security 2017-09-15 13:26:33 UTC
CVE-2017-14484 assigned.

@Security please add it to the database.


Gentoo Security Padawan
ChrisADR
Comment 29 GLSAMaker/CVETool Bot gentoo-dev 2017-09-17 19:06:37 UTC
This issue was resolved and addressed in
 GLSA 201709-11 at https://security.gentoo.org/glsa/201709-11
by GLSA coordinator Aaron Bauman (b-man).