PERFORCE change 32072 for review

Adam Migus adam at migus.org
Thu May 29 23:47:03 GMT 2003


Robert,
Sorry I failed to address a point.  I assumed when
writing the code that the caller would pass in a large
enough string, as that assumption is made elsewhere in
code called at this layer in the framework.  To be
explicit, I think this change is therefor rendundant.
Also I am curious, asside from intentionally passing
in a string that's too short to hold the result can
you make this code panic?

Adam

<quote who="Adam Migus">
> Robert,
> len can never exceed size which is passed in as the
> size of the string.  Therefor the NULL will always
> land inside the string.
> How exactly does snprintf() fail?
>
> Adam
>
> <quote who="Robert Watson">
>> http://perforce.freebsd.org/chv.cgi?CH=32072
>>
>> Change 32072 by rwatson at rwatson_tislabs on
>> 2003/05/29 16:14:02
>>
>> 	Temporary work-around for overflows in
>> externalization of
>> 	compartment strings in the Biba and MLS policies.
>> Validate
>> 	that the nul we slap down in fact lands inside
>> the
>> string.
>> 	This code generally needs cleaning up, since it
>> fails to
>> 	handle failures by snprintf().  If the provided
>> string is
>> 	too short, this result is preferable to kernel
>> panics, etc.
>>
>> Affected files ...
>>
>> ..
>> //depot/projects/trustedbsd/mac/sys/security/mac_biba/mac_biba.c#209
>> edit ..
>> //depot/projects/trustedbsd/mac/sys/security/mac_mls/mac_mls.c#167
>> edit
>>
>> Differences ...
>>
>> ====
>> //depot/projects/trustedbsd/mac/sys/security/mac_biba/mac_biba.c#209
>> (text+ko) ====
>>
>> @@ -583,7 +583,11 @@
>>  		} while(bit <= MAC_BIBA_MAX_COMPARTMENTS);
>>
>>  		len = size - left - 1;
>> -		string[len] = '\0';
>> +		if (len > 0 && len < size)
>> +			string[len] = '\0';
>> +		else
>> +			string[0] = '\0';
>> +
>>  		return (len);
>>
>>  	default:
>>
>> ====
>> //depot/projects/trustedbsd/mac/sys/security/mac_mls/mac_mls.c#167
>> (text+ko) ====
>>
>> @@ -547,7 +547,10 @@
>>  		} while(bit <= MAC_MLS_MAX_COMPARTMENTS);
>>
>>  		len = size - left - 1;
>> -		string[len] = '\0';
>> +		if (len > 0 && len < size)
>> +			string[len] = '\0';
>> +		else
>> +			string[0] = '\0';
>>  		return (len);
>>
>>  	default:
>
>
> --
> Adam Migus - Research Scientist
> Network Associates Laboratories
> (http://www.nailabs.com)
> FreeBSD (http://www.freebsd.org)
> God may be subtle, but He isn't plain mean.  --
> Albert
> Einstein


-- 
Adam Migus - Research Scientist
Network Associates Laboratories (http://www.nailabs.com)
FreeBSD (http://www.freebsd.org)
God may be subtle, but He isn't plain mean.  -- Albert
Einstein



To Unsubscribe: send mail to majordomo at trustedbsd.org
with "unsubscribe trustedbsd-cvs" in the body of the message



More information about the trustedbsd-cvs mailing list