Messages in this thread | | | Date | Thu, 19 Oct 2023 10:40:02 +0700 | From | Bagas Sanjaya <> | Subject | Re: Is strncpy really less secure than strscpy ? |
| |
On Wed, Oct 18, 2023 at 07:56:36PM -0700, Kees Cook wrote: > On Wed, Oct 18, 2023 at 07:27:20PM -0700, Randy Dunlap wrote: > > > > > > On 10/18/23 18:49, Bagas Sanjaya wrote: > > > [Disclaimer: I have little to no knowledge of C, so things may be wrong. > > > Please correct me if it is the case. Also Cc: recent people who work on > > > strscpy() conversion.] > > Here are the current docs on the deprecated use of strncpy: > https://docs.kernel.org/process/deprecated.html#strncpy-on-nul-terminated-strings > which could probably be improved. > > > Also Cc: the STRING maintainers. > > > > > On Thu, Oct 19, 2023 at 12:22:33AM +0100, James Dutton wrote: > > >> Is strncpy really less secure than strscpy ? > > Very. :) > > > >> If one uses strncpy and thus put a limit on the buffer size during the > > >> copy, it is safe. There are no writes outside of the buffer. > > >> If one uses strscpy and thus put a limit on the buffer size during the > > >> copy, it is safe. There are no writes outside of the buffer. > > > > > > Well, assuming that the string is NUL-terminated, the end result should > > > be the same. > > Note the use of "If" in the above sentences. :) This is what makes > strncpy so dangerous -- it's only "correct" if the length argument is > less than the size of the _source_ buffer. And by "correct", I mean > that only then will strncpy produce a C-string. If not, it's a memcpy > and leaves the buffer unterminated. This lack of %NUL-termination leads > to all kinds of potential "downstream" problems with reading past the > end of the buffer, etc.
Oh, that's what I mean by the same results.
> > One of the easiest ways to avoid bugs is to remove ambiguity, and > strncpy is full of it. :P > > Almost more important than the potential lack of %NUL-termination is > the fact that strncpy() doesn't tell other maintainers _why_ it was used > because it has several "effects" that aren't always intended: > > - is the destination supposed to be %NUL terminated? (We covered this) > - is the destination supposed to be %NUL padded? > > strncpy pads the destination -- is this needed? Is it a waste of CPU > time? > > > > > > >> But, one can fit more characters in strncpy than strscpy because > > >> strscpy enforces the final \0 on the end. > > >> One could argue that strncpy is better because it might save the space > > >> of one char at the end of a string array. > > At the cost of creating non-C-strings. And this is a completely bonkers > result for the "C String API" to produce. :P > > > >> There are cases where strncpy might be unsafe. For example copying > > >> between arrays of different sizes, and that is a case where strscpy > > >> might be safer, but strncpy can be made safe if one ensures that the > > >> size used in strncpy is the smallest of the two different array sizes. > > The CONFIG_FORTIFY_SOURCE option in the kernel adds a bunch of > sanity-checking to the APIs (some of which can be determined at compile > time), but it doesn't remove the ambiguity of using strncpy. We want the > kernel to have maintainable code, and when it's not clear which of a > handful of side-effects are _intended_ from an API, that's a bad API. :) > > > > > > > Code example on both cases? > > > > > >> > > >> If one blindly replaces strncpy with strscpy across all uses, one > > >> could unintentionally be truncating the results and introduce new > > >> bugs. > > Yes, of course. That's why we're not blindly replacing them. :) And the > diagnostic criteria has been carefully described: > https://github.com/KSPP/linux/issues/90 >
Thanks for the explanation!
-- An old man doll... just what I always wanted! - Clara [unhandled content-type:application/pgp-signature] | |