Date: Thu, 29 May 2003 19:47:03 -0400 (EDT) From: "Adam Migus" <adam@migus.org> To: <adam@migus.org> Cc: rwatson@freebsd.org Subject: Re: PERFORCE change 32072 for review Message-ID: <45980.205.227.136.1.1054252023.squirrel@mail.migus.org> In-Reply-To: <41432.205.227.136.1.1054251755.squirrel@mail.migus.org> References: <200305292314.h4TNEYbs070967@repoman.freebsd.org> <41432.205.227.136.1.1054251755.squirrel@mail.migus.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?45980.205.227.136.1.1054252023.squirrel>