On Tue, Aug 08, 2023 at 10:48:11PM +0000, Justin Stitt wrote: > Prefer `strscpy` to `strncpy` for use on NUL-terminated destination > buffers. > > This fixes a potential bug due to the fact that both `t->u.user.name` > and `name` share the same size. > > Signed-off-by: Justin Stitt <justinstitt@xxxxxxxxxx> > > --- > Here's an example of what happens when dest and src share same size: > | #define MAXLEN 5 > | char dest[MAXLEN]; > | const char *src = "hello"; > | strncpy(dest, src, MAXLEN); // -> should use strscpy() > | // dest is now not NUL-terminated > --- > net/netfilter/x_tables.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c > index 470282cf3fae..714a38ec9055 100644 > --- a/net/netfilter/x_tables.c > +++ b/net/netfilter/x_tables.c > @@ -768,7 +768,7 @@ void xt_compat_match_from_user(struct xt_entry_match *m, void **dstptr, > m->u.user.match_size = msize; > strscpy(name, match->name, sizeof(name)); > module_put(match->me); > - strncpy(m->u.user.name, name, sizeof(m->u.user.name)); > + strscpy(m->u.user.name, name, sizeof(m->u.user.name)); Two hints here are that this is dealing with user-space memory copies, so NUL-padding is needed (i.e. don't leak memory contents), and immediately above is a strscpy() already, which means there is likely some reason strncpy() is needed (i.e. padding). -- Kees Cook
