| Message ID | 20230808-net-netfilter-v1-7-efbbe4ec60af@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | netfilter: refactor deprecated strncpy | expand |
On Wednesday 2023-08-09 00:48, Justin Stitt wrote: >Prefer `strscpy` as it's a more robust interface. > >There may have existed a bug here due to both `tbl->repl.name` and >`info->name` having a size of 32 as defined below: >| #define XT_TABLE_MAXNAMELEN 32 > >This may lead to buffer overreads in some situations -- `strscpy` solves >this by guaranteeing NUL-termination of the dest buffer. It generally will not lead to overreads. xt not only deals with strings on its own turf, it even takes them from userspace-provided buffers, which means extra scrutiny is absolutely required. Done in places like x_tables.c: if (strnlen(name, XT_EXTENSION_MAXNAMELEN) == XT_EXTENSION_MAXNAMELEN) (Which is not to say the strncpy->strscpy mop-up is bad.)
diff --git a/net/netfilter/xt_repldata.h b/net/netfilter/xt_repldata.h index 68ccbe50bb1e..63869fd0ec57 100644 --- a/net/netfilter/xt_repldata.h +++ b/net/netfilter/xt_repldata.h @@ -29,7 +29,7 @@ if (tbl == NULL) \ return NULL; \ term = (struct type##_error *)&(((char *)tbl)[term_offset]); \ - strncpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ + strscpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ *term = (struct type##_error)typ2##_ERROR_INIT; \ tbl->repl.valid_hooks = hook_mask; \ tbl->repl.num_entries = nhooks + 1; \
Prefer `strscpy` as it's a more robust interface. There may have existed a bug here due to both `tbl->repl.name` and `info->name` having a size of 32 as defined below: | #define XT_TABLE_MAXNAMELEN 32 This may lead to buffer overreads in some situations -- `strscpy` solves this by guaranteeing NUL-termination of the dest buffer. Signed-off-by: Justin Stitt <justinstitt@google.com> --- Note: build tested only --- net/netfilter/xt_repldata.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)