Is strncpy really less secure than strscpy ?
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.
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.
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.
If one blindly replaces strncpy with strscpy across all uses, one
could unintentionally be truncating the results and introduce new
bugs.
The real insecurity surely comes when one tries to use the string.
For example:
#include <stdio.h>
#include <string.h>
int main() {
char a[10] = "HelloThere";
char b[10];
char c[10] = "Overflow";
strncpy(b, a, 10);
/* This overflows and so in unsafe */
printf("a is %s\n", a);
/* This overflows and so in unsafe */
printf("b is %s\n", b);
/* This is safe */
printf("b is %.*s\n", 10, a);
/* This is safe */
printf("b is %.*s\n", 4, a);
return 0;
}
So, why isn't the printk format specifier "%.*s" used more instead of
"%s" in the kernel?
Kind Regards
James
[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.]
On Thu, Oct 19, 2023 at 12:22:33AM +0100, James Dutton wrote:
> Is strncpy really less secure than strscpy ?
>
> 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.
> 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.
> 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.
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.
>
> The real insecurity surely comes when one tries to use the string.
> For example:
>
> #include <stdio.h>
> #include <string.h>
>
> int main() {
> char a[10] = "HelloThere";
> char b[10];
> char c[10] = "Overflow";
> strncpy(b, a, 10);
> /* This overflows and so in unsafe */
> printf("a is %s\n", a);
> /* This overflows and so in unsafe */
> printf("b is %s\n", b);
> /* This is safe */
> printf("b is %.*s\n", 10, a);
> /* This is safe */
> printf("b is %.*s\n", 4, a);
> return 0;
> }
What if printf("a is %.*s\n", a);?
>
>
> So, why isn't the printk format specifier "%.*s" used more instead of
> "%s" in the kernel?
Since basically strings are pointers.
Thanks.
--
An old man doll... just what I always wanted! - Clara
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.]
>
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 ?
>>
>> 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.
>
>> 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.
>> 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.
>
> 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.
>>
>> The real insecurity surely comes when one tries to use the string.
>> For example:
>>
>> #include <stdio.h>
>> #include <string.h>
>>
>> int main() {
>> char a[10] = "HelloThere";
>> char b[10];
>> char c[10] = "Overflow";
>> strncpy(b, a, 10);
>> /* This overflows and so in unsafe */
>> printf("a is %s\n", a);
>> /* This overflows and so in unsafe */
>> printf("b is %s\n", b);
>> /* This is safe */
>> printf("b is %.*s\n", 10, a);
>> /* This is safe */
>> printf("b is %.*s\n", 4, a);
>> return 0;
>> }
>
> What if printf("a is %.*s\n", a);?
>
>>
>>
>> So, why isn't the printk format specifier "%.*s" used more instead of
>> "%s" in the kernel?
>
> Since basically strings are pointers.
>
> Thanks.
>
--
~Randy
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.
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
-Kees
--
Kees Cook
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
On Wed, Oct 18, 2023 at 7:56 PM Kees Cook <[email protected]> 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.
>
> 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:
THIS.
It is often very difficult and takes multiple minutes to figure out whether
a destination buffer used within strncpy() is expected to be NUL-terminated
or NUL-padded. The behavior and intention of other viable replacements
(such as strscpy()) are immediately obvious to the reader.
>
> - 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
But yeah, this is the kicker. C String API's should produce C strings.
>
> > >> 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
>
> -Kees
>
> --
> Kees Cook
On Thu, 19 Oct 2023 at 02:49, Bagas Sanjaya <[email protected]> wrote:
>
[snip]
>
> What if printf("a is %.*s\n", a);?
Um, it fails to compile.
>
> >
> >
> > So, why isn't the printk format specifier "%.*s" used more instead of
> > "%s" in the kernel?
>
> Since basically strings are pointers.
Um, I was trying to draw people's attention to the fact that "%.*s" is
much safer than "%s".
"%s" is like strcpy() but for print statements.
"%.*s" is like strncpy() but for print statements.
Why wasn't "%.*s" also included in the string discussions previously?