CVS update: samba/source

Alexander Bokovoy a.bokovoy at sam-solutions.net
Wed Sep 10 16:39:02 GMT 2003


On Thu, Sep 11, 2003 at 12:55:32AM +1000, Andrew Bartlett wrote:
> > > Now that CAN-2003-0689 is published officially, we need to make possible
> > > to build on systems with fixed getgrouplist() in GNU libc < 2.3.2.
> > > Unfortunately, we can't detect correctness of getgrouplist() functioning
> > > in portable way so this is left up to developer/packager.
> > > 
> > > This patch adds --with-good-getgrouplist[=no] switch to configure which
> > > packagers on Linux platforms could use to specify in their own builds if
> > > they now that glibc on their platform is fixed w.r.t CAN-2003-0689. By
> > > default we still think that glibc is vulnerable and perform our version
> > > check.
> > 
> > Why do we have to add another configure option?  It takes forever to gid 
> > rid of these things.  If a package maintainer kno9ws that glibc is ok on 
> > their platform, then they can patch the source (samba).  That's what linux 
> > distros do anyways.
> > 
> > I really want to revert this change but will wait to see if other people
> > agree.
> > 
> > Sorry.  I should have spoken up in response to your original mail.
> 
> I agree - these take forever, and those that 'know' their system is fine
> (and only a packager can really know that, as they can assert a
> dependency) can patch. 
> 
> Otherwise, we have a config option for years - but worse we have people
> who will turn it on/off on all sorts of systems, without understanding
> it.
Ok, made a test which seems to work and detects vulnerable systems. 
The idea is to pass big buffer to getgrouplist() but request 0 groups to
return. This way even primary group copying should fail on proper system
and copy nothing thus resulting in proper behaviour. On vulnerable systems
it will copy all groups but as we have passed much bigger buffer (I think
4096 groups would be enough), this wouldn't cause seg fault in our test.

-- 
/ Alexander Bokovoy
---
No one regards what is before his feet; we all gaze at the stars.
		-- Quintus Ennius
-------------- next part --------------
#include <sys/types.h>
#include <unistd.h>
#include <stdio.h>

#define TEST_NGROUPS 4096
#define TEST_REQUESTED_GROUPS 0
int main() {

	int ngroups = TEST_REQUESTED_GROUPS;
	unsigned long i,count;
	
	gid_t *groups = (gid_t *) malloc (TEST_NGROUPS * sizeof (gid_t));

	if (groups == NULL) {
		fprintf(stderr, "Error: can't allocate storage for %d groups\n", TEST_NGROUPS);
		return 1;
	}
	
	for(i=0; i<TEST_NGROUPS; i++) groups[i] = (gid_t) -1;
	
	if (getgrouplist ("root", 0, groups, &ngroups) >= 0) {
		/* Situation was correctly handled, return success */
		free(groups);
		fprintf(stderr, "Success: %d group(s) were returned\n", ngroups);
		return 0;
	}
	
	/* Ok, there was an error -- passed array was too small 
	   Now ngroups contains actual number of copied groups
	*/

	count = 0;
	for(i=0; i<TEST_NGROUPS; i++) {
		if (groups[i] == (gid_t) -1) count++;
	}
	if (TEST_REQUESTED_GROUPS != TEST_NGROUPS-count) {
		fprintf(stderr, "Error: returned %d group(s) for requested %d\n", ngroups, TEST_REQUESTED_GROUPS);
		return 1;
	}
	
	free(groups);

	fprintf(stderr, "Success: actually copied %d group(s) despite reporting of %d\n", TEST_NGROUPS-count, ngroups);
	return 0;
}


More information about the samba-technical mailing list