pam_motd runs commands as root with unsanitised environment

@Dustin, you removed yourself from the assignment even though aiui you implemented the feature. Can you provide more details?

Jamie,

I'm attaching the patch that I've tested and verified works here for me. I sanitize the environment using env -i, and hardcode PATH to the values I pulled from /etc/login.defs:ENV_SUPATH. Arguably, there might be a few other environment values we should pass here (LANG?). I also hardcoded the path of /bin/run-parts.

I discussed this with someone in IRC (Kees? Steve? You? Sorry, I don't recall who) back in December 2010, and whomever I showed this to didn't like it. They wanted a fork/exec approach instead, which I didn't have time to implement. At that point, I unassigned myself from the bug.

To test, I added $HOME/bin to the path of user 'kirkland'. I added a shell script, $HOME/bin/uname which does a "date >> /root/howdy". I then added "session optional pam_motd.so" to the end of /etc/pam.d/su. All of this as the reported suggested.

Before applying my patch, I would su and definitely see the file /root/howdy created (verifying the vulnerability). After applying and installing my patch, I would not see /root/howdy created. As far as I could tell, the rest of the update-motd part of pam_motd seemed to work correctly without regression.

If the attached patch is acceptable, then feel free to assign this bug back to me, and I'll prepare SRUs, and upload to Oneiric.

If the attached patch is unacceptable, then I politely request that the dissatisfied parties attached a preferred diff.

Thanks,
:-Dustin

On Wed, Apr 27, 2011 at 06:23:28PM -0000, Dustin Kirkland wrote:
> I discussed this with someone in IRC (Kees? Steve? You? Sorry, I
> don't recall who) back in December 2010, and whomever I showed this to
> didn't like it. They wanted a fork/exec approach instead, which I
> didn't have time to implement. At that point, I unassigned myself from
> the bug.

Wasn't me; I'm not qualified to have an opinion on this. :)

--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developer http://www.debian.org/
<email address hidden> <email address hidden>

Forking was a suggestion for improving login speed (fork update-motd.d scripts) and wasn't related to this. I think the attached patch should be fine.

It would be nice to update the manpages for Oneiric too.

This bug was fixed in the package pam - 1.1.2-2ubuntu9

---------------
pam (1.1.2-2ubuntu9) oneiric; urgency=low

  * debian/patches-applied/update-motd: santize the environment before
    calling run-parts, LP: #610125
 -- Dustin Kirkland <email address hidden> Wed, 27 Apr 2011 13:02:15 -0500

 - SRU justification added.
 - Uploaded to Oneiric
 - Uploaded to [lucid-proposed, maverick-proposed, natty-proposed]
 - Subscribed ubuntu-sru
 - Awaiting ack from ubuntu-sru, and acceptance of queued packages

Looks good, I'm accepting them now. One question, since its already been mentioned in an oneiric upload, when does this bug go from private -> public ?

Accepted pam into natty-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

Accepted pam into maverick-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

Accepted pam into lucid-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

I don't see any reason why this bug really needs to be private
necessarily, from my PoV.

This really shouldn't be an SRU, but a security update. Now that the update is in -proposed, it's public and thus the bug should be public as well, making so.

Since this is already part of the SRU process, let's just follow that and when it is accepted, the security team can do a no change rebuild. Alternatively, at the time the SRU team would accept it, we can do a no change rebuild in -security and push updates out then (to reduce archive churn and pain do to pam service restarts).

I will be releasing pam security updates in a week or so, so I will bundle this issue with them.

This still needs fixing, unfortunately. "env" is called without a fully qualified path, which means a malicious PATH can still cause problems. (Again, only in the case of having pam_motd added to non-default pam service configs that are local setuid applications.)

Proposed patch attached.

Strangely, this patch was dropped from natty and oneiric. What happened?

Looks like another UDD casualty:

http://bazaar.launchpad.net/~ubuntu-branches/ubuntu/oneiric/pam/oneiric/revision/79#debian/changelog

I still see the patch in natty, so looks like it's only missing in Oneiric.
Thanks for the fix Dustin, I'll push it out as a security update once I get a proper CVE.

On Thu, Oct 13, 2011 at 11:05:46PM -0000, Dustin Kirkland wrote:
> Strangely, this patch was dropped from natty and oneiric. What
> happened?

The patch was not committed to the Vcs-Bzr branch that's documented as
authoritative in the package's debian/control file, so the change was
dropped in the next merge from Debian. Please make sure to push your
changes to lp:~ubuntu-core-dev/pam/ubuntu.

--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developer http://www.debian.org/
<email address hidden> <email address hidden>

Thanks Marc.

This is CVE-2011-3628.

Thank you for reporting this bug to Ubuntu. maverick has reached EOL
(End of Life) and is no longer supported. As a result, this bug
against maverick is being marked "Won't Fix". Please see
https://wiki.ubuntu.com/Releases for currently supported Ubuntu
releases.

Please feel free to report any other bugs you may find.

Thank you for reporting this bug to Ubuntu. natty has reached EOL
(End of Life) and is no longer supported. As a result, this bug
against natty is being marked "Won't Fix". Please see
https://wiki.ubuntu.com/Releases for currently supported Ubuntu
releases.

Please feel free to report any other bugs you may find.

Could we get an update here, please? There's a fix available for almost one and a half years now.

This looks fixed in raring, precise, oneiric, and lucid:

$ grep -r env.*run-parts:

./pam-1.1.3/debian/patches-applied/update-motd:+ if (!system("/usr/bin/env -i PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin run-parts --lsbsysinit /etc/update-motd.d > /var/run/motd.new"))
./pam-1.1.3/debian/patches-applied/update-motd:+ if (!system("/usr/bin/env -i PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin run-parts --lsbsysinit /etc/update-motd.d > /var/run/motd.new"))
./pam-1.1.3/debian/patches-applied/update-motd:+ if (!system("/usr/bin/env -i PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin run-parts --lsbsysinit /etc/update-motd.d > /var/run/motd.new"))
pam-1.1.1/debian/patches-applied/update-motd:+ if (!system("/usr/bin/env -i PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin /bin/run-parts --lsbsysinit /etc/update-motd.d > /var/run/motd.new"))