See thread on oss-security mailing list [1] for additional reference. This issue has been assigned CVE-2011-1089. Essentially every setuid mount helper, including util-linux mount, fails to handle a low RLIMIT_FSIZE. Since addmntent() uses fprintf() without a corresponding fflush(), it will return success even in the case when the actual write will fail due to the resource limit. And then endmntent() always returns 1, so that's no help. As a result, these helpers can be used to write corrupted entries into /etc/mtab. Rather than forcing every setuid mount helper to explicitly alter resource limits prior to calling addmntent(), etc., it would be better if addmntent() attempted to flush and return failure based on the success of the fflush(), as suggested by Tomas Hoger: if (fprintf (stream, "%s %s %s %s %d %d\n", ...) < 0) return 1; return (fflush(stream) == 0 ? 0 : 1); [1] http://www.openwall.com/lists/oss-security/2011/03/04/9 [2] http://www.openwall.com/lists/oss-security/2011/03/07/9
Created attachment 5346 [details] Simple test case gmane thread view may offer a better overview over the discussion Dan pointed out: http://thread.gmane.org/gmane.comp.security.oss.general/4374
(In reply to comment #0) > Rather than forcing every setuid mount helper to explicitly alter resource > limits prior to calling addmntent(), etc., it would be better if addmntent() > attempted to flush and return failure based on the success of the fflush(), as > suggested by Tomas Hoger: That's completely bogus. Applications which cannot handle problems introduced like this also won't check the return value of addmntent. Just fix the damned programs which doesn't use the existing interface correctly.
(In reply to comment #2) > (In reply to comment #0) > > Rather than forcing every setuid mount helper to explicitly alter resource > > limits prior to calling addmntent(), etc., it would be better if addmntent() > > attempted to flush and return failure based on the success of the fflush(), as > > suggested by Tomas Hoger: > > That's completely bogus. Applications which cannot handle problems introduced > like this also won't check the return value of addmntent. This bug is not about fixing apps that don't check addmntent return value via some glibc-side magic, it is about fixing addmntent to not return success when mtab update failed (or is going to fail when the buffer is flushed in endmntent / fclose), so the applications that do check return value can detect the error. > Just fix the damned programs which doesn't use the existing interface correctly. Can you clarify what exactly you refer to as incorrect use of existing interface? Even if you check addmntent return value, you can't rely on it. endmntent always returns the same value, hence no check is possible there.
Reopening per previous comment; even applications that properly use the existing interface (I assume you mean that they check addmntent() return value) wouldn't detect the mtab corruption.
The only reason I made a change is because some braindead idiot put in the man page that endmntent never returns anything but 1. This is not what the libc implementation says and it should have been the correct way to report this type of problem.
Commit link noted for posterity: http://sourceware.org/git/?p=glibc.git;a=commitdiff;h=e1fb097f44 (In reply to comment #5) > The only reason I made a change is because some braindead idiot put in the man > page that endmntent never returns anything but 1. This is not what the libc > implementation says and it should have been the correct way to report this type > of problem. Yes, it would be cleaner to have endmntent return error when fclose fails, but given the documentation, it's not easy to change that way. The man page seems to correctly document the implementation: http://sourceware.org/git/?p=glibc.git;a=blob;f=misc/mntent_r.c;h=6959f0e2#l58
(In reply to comment #6) > The man page seems > to correctly document the implementation: That's no excuse. Implementations change. The glibc documentation always documented it correctly. The person writing that text should be held accountable for the crap s/he did.
On Fri, 13 Jun 2014, fweimer at redhat dot com wrote: > Florian Weimer <fweimer at redhat dot com> changed: > > What |Removed |Added > ---------------------------------------------------------------------------- > Flags| |security- Are you sure? This has a CVE....
Fixed in this commit: http://sourceware.org/git/?p=glibc.git;a=commitdiff;h=e1fb097f447a89