linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Is strncpy really less secure than strscpy ?
@ 2023-10-18 23:22 James Dutton
  2023-10-19  1:49 ` Bagas Sanjaya
  0 siblings, 1 reply; 7+ messages in thread
From: James Dutton @ 2023-10-18 23:22 UTC (permalink / raw)
  To: LKML Mailing List

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Is strncpy really less secure than strscpy ?
  2023-10-18 23:22 Is strncpy really less secure than strscpy ? James Dutton
@ 2023-10-19  1:49 ` Bagas Sanjaya
  2023-10-19  2:27   ` Randy Dunlap
  0 siblings, 1 reply; 7+ messages in thread
From: Bagas Sanjaya @ 2023-10-19  1:49 UTC (permalink / raw)
  To: James Dutton, Linux Kernel Mailing List
  Cc: Justin Stitt, Calvince Otieno, Azeem Shaikh

[-- Attachment #1: Type: text/plain, Size: 2244 bytes --]

[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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Is strncpy really less secure than strscpy ?
  2023-10-19  1:49 ` Bagas Sanjaya
@ 2023-10-19  2:27   ` Randy Dunlap
  2023-10-19  2:56     ` Kees Cook
  2023-10-19 18:13     ` James Dutton
  0 siblings, 2 replies; 7+ messages in thread
From: Randy Dunlap @ 2023-10-19  2:27 UTC (permalink / raw)
  To: Bagas Sanjaya, James Dutton, Linux Kernel Mailing List
  Cc: Justin Stitt, Calvince Otieno, Azeem Shaikh, Kees Cook, Andy Shevchenko



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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Is strncpy really less secure than strscpy ?
  2023-10-19  2:27   ` Randy Dunlap
@ 2023-10-19  2:56     ` Kees Cook
  2023-10-19  3:40       ` Bagas Sanjaya
  2023-10-19 17:09       ` Justin Stitt
  2023-10-19 18:13     ` James Dutton
  1 sibling, 2 replies; 7+ messages in thread
From: Kees Cook @ 2023-10-19  2:56 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Bagas Sanjaya, James Dutton, Linux Kernel Mailing List,
	Justin Stitt, Calvince Otieno, Azeem Shaikh, Andy Shevchenko

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Is strncpy really less secure than strscpy ?
  2023-10-19  2:56     ` Kees Cook
@ 2023-10-19  3:40       ` Bagas Sanjaya
  2023-10-19 17:09       ` Justin Stitt
  1 sibling, 0 replies; 7+ messages in thread
From: Bagas Sanjaya @ 2023-10-19  3:40 UTC (permalink / raw)
  To: Kees Cook, Randy Dunlap
  Cc: James Dutton, Linux Kernel Mailing List, Justin Stitt,
	Calvince Otieno, Azeem Shaikh, Andy Shevchenko

[-- Attachment #1: Type: text/plain, Size: 3772 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Is strncpy really less secure than strscpy ?
  2023-10-19  2:56     ` Kees Cook
  2023-10-19  3:40       ` Bagas Sanjaya
@ 2023-10-19 17:09       ` Justin Stitt
  1 sibling, 0 replies; 7+ messages in thread
From: Justin Stitt @ 2023-10-19 17:09 UTC (permalink / raw)
  To: Kees Cook
  Cc: Randy Dunlap, Bagas Sanjaya, James Dutton,
	Linux Kernel Mailing List, Calvince Otieno, Azeem Shaikh,
	Andy Shevchenko

On Wed, Oct 18, 2023 at 7:56 PM Kees Cook <keescook@chromium.org> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Is strncpy really less secure than strscpy ?
  2023-10-19  2:27   ` Randy Dunlap
  2023-10-19  2:56     ` Kees Cook
@ 2023-10-19 18:13     ` James Dutton
  1 sibling, 0 replies; 7+ messages in thread
From: James Dutton @ 2023-10-19 18:13 UTC (permalink / raw)
  To: Bagas Sanjaya, Linux Kernel Mailing List
  Cc: Justin Stitt, Randy Dunlap, Calvince Otieno, Azeem Shaikh,
	Kees Cook, Andy Shevchenko

On Thu, 19 Oct 2023 at 02:49, Bagas Sanjaya <bagasdotme@gmail.com> 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?

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-10-19 17:57 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-18 23:22 Is strncpy really less secure than strscpy ? James Dutton
2023-10-19  1:49 ` Bagas Sanjaya
2023-10-19  2:27   ` Randy Dunlap
2023-10-19  2:56     ` Kees Cook
2023-10-19  3:40       ` Bagas Sanjaya
2023-10-19 17:09       ` Justin Stitt
2023-10-19 18:13     ` James Dutton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).