| Message ID | 20201222222356.22645-1-fw@strlen.de (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [nf] netfilter: xt_RATEEST: reject non-null terminated string from userspace | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | warning | 6 maintainers not CCed: davem@davemloft.net coreteam@netfilter.org kaber@trash.net kuba@kernel.org kadlec@netfilter.org pablo@netfilter.org |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 9 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Tue, Dec 22, 2020 at 2:24 PM Florian Westphal <fw@strlen.de> wrote: > > strlcpy assumes src is a c-string. Check info->name before its used. If strlcpy is the only problem, then the fix is to use strscpy(), which doesn't have the design mistake that strlcpy has. Of course, if the size limit of the source and the destination differ (ie if you really want to limit the source to one thing, and the destination to another - there are in theory valid cases where that happens), then there are no useful helper functions for that. Linus
Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Tue, Dec 22, 2020 at 2:24 PM Florian Westphal <fw@strlen.de> wrote: > > > > strlcpy assumes src is a c-string. Check info->name before its used. > > If strlcpy is the only problem, then the fix is to use strscpy(), > which doesn't have the design mistake that strlcpy has. It would silence the reproducer, but the checkentry function calls __xt_rateest_lookup which may 'strcmp(..., maybe_not_zero_terminated)'.
On Tue, Dec 22, 2020 at 11:23:56PM +0100, Florian Westphal wrote: > syzbot reports: > detected buffer overflow in strlen > [..] > Call Trace: > strlen include/linux/string.h:325 [inline] > strlcpy include/linux/string.h:348 [inline] > xt_rateest_tg_checkentry+0x2a5/0x6b0 net/netfilter/xt_RATEEST.c:143 > > strlcpy assumes src is a c-string. Check info->name before its used. Applied, thanks Florian.
diff --git a/net/netfilter/xt_RATEEST.c b/net/netfilter/xt_RATEEST.c index 37253d399c6b..0d5c422f8745 100644 --- a/net/netfilter/xt_RATEEST.c +++ b/net/netfilter/xt_RATEEST.c @@ -115,6 +115,9 @@ static int xt_rateest_tg_checkentry(const struct xt_tgchk_param *par) } cfg; int ret; + if (strnlen(info->name, sizeof(est->name)) >= sizeof(est->name)) + return -ENAMETOOLONG; + net_get_random_once(&jhash_rnd, sizeof(jhash_rnd)); mutex_lock(&xn->hash_lock);
syzbot reports: detected buffer overflow in strlen [..] Call Trace: strlen include/linux/string.h:325 [inline] strlcpy include/linux/string.h:348 [inline] xt_rateest_tg_checkentry+0x2a5/0x6b0 net/netfilter/xt_RATEEST.c:143 strlcpy assumes src is a c-string. Check info->name before its used. Reported-by: syzbot+e86f7c428c8c50db65b4@syzkaller.appspotmail.com Fixes: 5859034d7eb8793 ("[NETFILTER]: x_tables: add RATEEST target") Signed-off-by: Florian Westphal <fw@strlen.de> --- RATEEST test in iptables.git still passes, syzbot repro setsockopt fails with -ENAMETOOLONG.