Re: [PATCH nf] netfilter: xt_CT: Force user-space strings as null terminated

2018-05-29 Thread Jozsef Kadlecsik
Hi,

On Tue, 29 May 2018, Pablo Neira Ayuso wrote:

> On Tue, May 29, 2018 at 11:58:29AM +0800, gfree.w...@vip.163.com wrote:
> > From: Gao Feng 
> > 
> > The helper and timeout strings are from user-space, we need to make
> > sure they are null terminated. If not, evil user could make kernel
> > read the unexpected memory, even print it when fail to find by the
> > following codes.
> > 
> > pr_info_ratelimited("No such helper \"%s\"\n", helper_name);
> > 
> > Signed-off-by: Gao Feng 
> > ---
> >  net/netfilter/xt_CT.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c
> > index 8790190..f4b7d31 100644
> > --- a/net/netfilter/xt_CT.c
> > +++ b/net/netfilter/xt_CT.c
> > @@ -245,12 +245,14 @@ static int xt_ct_tg_check(const struct xt_tgchk_param 
> > *par,
> > }
> >  
> > if (info->helper[0]) {
> > +   info->helper[sizeof(info->helper) - 1] = '\0';
> 
> Probably better to reject non-nul terminated strings from userspace?

Attackers can easily build custom tools by which they then send arbitrary 
parameters to the kernel. So I think it's better to sanitize the input at 
kernel side - and do the same in our userspace tools as well :-)

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH nf] netfilter: xt_CT: Force user-space strings as null terminated

2018-05-29 Thread Pablo Neira Ayuso
On Tue, May 29, 2018 at 11:58:29AM +0800, gfree.w...@vip.163.com wrote:
> From: Gao Feng 
> 
> The helper and timeout strings are from user-space, we need to make
> sure they are null terminated. If not, evil user could make kernel
> read the unexpected memory, even print it when fail to find by the
> following codes.
> 
> pr_info_ratelimited("No such helper \"%s\"\n", helper_name);
> 
> Signed-off-by: Gao Feng 
> ---
>  net/netfilter/xt_CT.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c
> index 8790190..f4b7d31 100644
> --- a/net/netfilter/xt_CT.c
> +++ b/net/netfilter/xt_CT.c
> @@ -245,12 +245,14 @@ static int xt_ct_tg_check(const struct xt_tgchk_param 
> *par,
>   }
>  
>   if (info->helper[0]) {
> + info->helper[sizeof(info->helper) - 1] = '\0';

Probably better to reject non-nul terminated strings from userspace?
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH nf] netfilter: xt_CT: Force user-space strings as null terminated

2018-05-28 Thread gfree . wind
From: Gao Feng 

The helper and timeout strings are from user-space, we need to make
sure they are null terminated. If not, evil user could make kernel
read the unexpected memory, even print it when fail to find by the
following codes.

pr_info_ratelimited("No such helper \"%s\"\n", helper_name);

Signed-off-by: Gao Feng 
---
 net/netfilter/xt_CT.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c
index 8790190..f4b7d31 100644
--- a/net/netfilter/xt_CT.c
+++ b/net/netfilter/xt_CT.c
@@ -245,12 +245,14 @@ static int xt_ct_tg_check(const struct xt_tgchk_param 
*par,
}
 
if (info->helper[0]) {
+   info->helper[sizeof(info->helper) - 1] = '\0';
ret = xt_ct_set_helper(ct, info->helper, par);
if (ret < 0)
goto err3;
}
 
if (info->timeout[0]) {
+   info->timeout[sizeof(info->timeout) - 1] = '\0';
ret = xt_ct_set_timeout(ct, par, info->timeout);
if (ret < 0)
goto err4;
-- 
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html