[Pkg-shadow-devel] [PATCH] newuidmap, newgidmap: Correct the range size sanity check in get_map_ranges

[Eric W. Biederman]

Serge Hallyn <serge.hallyn at ubuntu.com> writes:

> Quoting Eric W. Biederman (ebiederm at xmission.com):
>> 
>> The number of ranges should be the ceiling of the number of arguments divided
>> by three.
>> 
>> Without this fix newuidmap and newgidmap always report and error and fail,
>> which is very much not what we want.
>
> Are you sure?

Certain enough that I tracked it down to that line.

> Without looking back at the code, I can say I've used
> them quite a bit and never seen an error.  When I originally looked
> at the code the -2 + 2 did annoy me, but I assumed it was supposed to
> remind you of something :)

Yes but then you copied that test to a location where argc - 2 was
already present.  So it is certainly a correct transformation.

> Can you give an example command line that fails for you?

The command line I was using when this triggered was:

newuidmap $child 0 $subuid 1000 $uid $uid 1

Let me check my math to make certain the problem checks out.
argc == 8 on entry into main.

argc - 2 (newuidmap and $child) == 6

Hmm.

You know I must have made a typo, and was short an argument when I was
testing that.   Ugh.

So perhaps things are not quite as bad as I made out.  But the fix is
definitely useful as otherwise we don't handle the case of being short
an argument or two correctly.  Which should hit the next test and
complain about being short arguments and should not complain that range
is incorrect.

Eric


>> Signed-off-by: "Eric W. Biederman" <ebiederm at xmission.com>
>> ---
>>  libmisc/idmapping.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/libmisc/idmapping.c b/libmisc/idmapping.c
>> index 81e85d5..714c29e 100644
>> --- a/libmisc/idmapping.c
>> +++ b/libmisc/idmapping.c
>> @@ -47,7 +47,7 @@ struct map_range *get_map_ranges(int ranges, int argc, char **argv)
>>  		return NULL;
>>  	}
>>  
>> -	if (ranges != ((argc - 2) + 2) / 3) {
>> +	if (ranges != ((argc + 2) / 3)) {
>>  		fprintf(stderr, "%s: ranges: %u is wrong for argc: %d\n", Prog, ranges, argc);
>>  		return NULL;
>>  	}
>> -- 
>> 1.7.10.4
>> 
>> 
>> _______________________________________________
>> Pkg-shadow-devel mailing list
>> Pkg-shadow-devel at lists.alioth.debian.org
>> http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-shadow-devel