[Pkg-samba-maint] Bug#568942: Bug#568942: samba: mtab corruption via malicious crafted string

[Steve Langasek]

severity 568942 important
found 568942 2:3.2.5-4
thanks

On Wed, Feb 10, 2010 at 08:00:28PM +0100, Moritz Muehlenhoff wrote:
> While there may be a patch for the specific issue, Jeremy made it pretty
> clear that it's not suitable for setuid root status. This second bug
> about the mtab corruption is another indicative.

In spite of Jeremy's strident insistence that the code hasn't been audited
(by whom? he doesn't say), it was clearly written (and not by him!) with
secure operation by root in mind.  TTBOMK, these are the only two security
issues that have been found in mount.cifs; the first is also an issue on any
system with mount points specified in /etc/fstab that are subdirectories of
user-controlled directories, and the second is documented as a denial of
service with no evidence of privilege escalation.

Minimizing the amount of suid code (and the amount of code running as root
generally) is important for security, but dropping the setuid bit on this
program in a stable release and breaking existing installations would be an
overreaction.

> While it's a little more intrusive than other fixes, it appears to me
> that the only correct fix for Lenny is also dropping the setuid root
> bit while documenting the necessary dpkg-statoverride calls.

I disagree.  That's not a correct fix, that's caving to FUD from samba
upstream.

Note that this mount helper originally had the setuid bit added because the
*upstream kernel documentation* indicated this was the correct way to
support per-user mounts; and for years before mount.cifs we were using
smbmount, which was also setuid-root and AFAIR had a similar audit status.

> I also fail to see why mount.cifs/umount.cifs should be accessible
> for a non-privileged user in the first place. Noone would even think
> about doing that for NFS, so why should CIFS be any different?

The difference is that unlike NFS, CIFS *mounts* are typically authenticated
using per-user, not per-system, credentials.  Nowadays FUSE may be a good
replacement for this, but that's not a reason to break the behavior of the
stable releases.

On Sat, Feb 13, 2010 at 09:32:43AM +0100, Christian PERRIER wrote:
> Steve, when discussing this, you were OK with dropping the setuid bit
> in squeeze (which we did...though I need now to upload) but at first
> glance, dropping it in lenny didn't have your favor. While I was
> originally having the same advice, I'm much more balanced right now,
> also because I looked at patches proposed in #6853 and I have doubts
> that my work on them to have them apply on Debian's 3.2.5 is correct.

The tarball attached to your earlier mail includes a number of patches that
are not related to bug #6853, and which have not been posted to bug #6853.
Where did you get this tarball?

In particular, the patches
0001-Revert-cifs-mount-did-not-properly-display-version-s.patch,
0002-s3-mount.cifs-make-mount.cifs-V-print-the-version-no.patch, and
0003-mount.cifs-directly-include-sys-stat.h-in-mtab.c.patch are unrelated to
either of the identified security issues and should not be applied to
stable; and 0004-mount.cifs-properly-check-for-mount-being-in-fstab-w.patch
and 0007-mount.cifs-don-t-allow-it-to-be-run-as-setuid-root-p.patch
deliberately change the behavior of mount.cifs with the rationale that
allowing users to mount shares on directories they own, or shipping
mount.cifs suid-root, is not "safe", which is upstream backpedalling on
previous design decisions and not related to either of the CVEs.

The only patches that are relevant for stable are
0005-mount.cifs-take-extra-care-that-mountpoint-isn-t-cha.patch and
0006-mount.cifs-check-for-invalid-characters-in-device-na.patch,
corresponding to CVE-2009-3297 and CVE-2010-0547 respectively.  I've applied
these to the lenny package and will be uploading to the lenny security queue
shortly.

-- 
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/
slangasek at ubuntu.com                                     vorlon at debian.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 828 bytes
Desc: Digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-samba-maint/attachments/20100213/cb92e29f/attachment-0001.pgp>