* [PATCH] ipv4: netfilter: ipt_CLUSTERIP: fix buffer overflow
@ 2011-03-10 18:12 Vasiliy Kulikov
2011-03-13 14:00 ` Changli Gao
0 siblings, 1 reply; 6+ messages in thread
From: Vasiliy Kulikov @ 2011-03-10 18:12 UTC (permalink / raw)
To: linux-kernel
Cc: security, Patrick McHardy, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, netfilter-devel, netfilter,
coreteam, netdev
buffer string is copied from userspace. It is not checked whether it is
zero terminated. This may lead to overflow inside of simple_strtoul().
It was introduced before the git epoch. Files "ipt_CLUSTERIP/*" are
root writable only by default, however, on some setups permissions might be
relaxed to e.g. network admin user.
Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
Compile tested.
net/ipv4/netfilter/ipt_CLUSTERIP.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c
index 403ca57..7aabf9a 100644
--- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
+++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
@@ -666,6 +666,7 @@ static ssize_t clusterip_proc_write(struct file *file, const char __user *input,
if (copy_from_user(buffer, input, PROC_WRITELEN))
return -EFAULT;
+ buffer[sizeof(buffer)-1] = 0;
if (*buffer == '+') {
nodenum = simple_strtoul(buffer+1, NULL, 10);
--
1.7.0.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] ipv4: netfilter: ipt_CLUSTERIP: fix buffer overflow
2011-03-10 18:12 [PATCH] ipv4: netfilter: ipt_CLUSTERIP: fix buffer overflow Vasiliy Kulikov
@ 2011-03-13 14:00 ` Changli Gao
2011-03-15 12:33 ` Patrick McHardy
0 siblings, 1 reply; 6+ messages in thread
From: Changli Gao @ 2011-03-13 14:00 UTC (permalink / raw)
To: Vasiliy Kulikov
Cc: linux-kernel, security, Patrick McHardy, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
netfilter-devel, netfilter, coreteam, netdev
On Fri, Mar 11, 2011 at 2:12 AM, Vasiliy Kulikov <segoon@openwall.com> wrote:
> buffer string is copied from userspace. It is not checked whether it is
> zero terminated. This may lead to overflow inside of simple_strtoul().
>
> It was introduced before the git epoch. Files "ipt_CLUSTERIP/*" are
> root writable only by default, however, on some setups permissions might be
> relaxed to e.g. network admin user.
>
> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
> ---
> Compile tested.
>
> net/ipv4/netfilter/ipt_CLUSTERIP.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c
> index 403ca57..7aabf9a 100644
> --- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
> +++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
> @@ -666,6 +666,7 @@ static ssize_t clusterip_proc_write(struct file *file, const char __user *input,
>
> if (copy_from_user(buffer, input, PROC_WRITELEN))
I think size should be used instead of PROC_WRITELEN.
if (size > PROC_WRITELEN)
return -EIO;
if (copy_from_user(buffer, input, size))
return -EFAULT;
buffer[size] = '\0';
> return -EFAULT;
> + buffer[sizeof(buffer)-1] = 0;
>
> if (*buffer == '+') {
> nodenum = simple_strtoul(buffer+1, NULL, 10);
--
Regards,
Changli Gao(xiaosuo@gmail.com)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ipv4: netfilter: ipt_CLUSTERIP: fix buffer overflow
2011-03-13 14:00 ` Changli Gao
@ 2011-03-15 12:33 ` Patrick McHardy
2011-03-17 11:32 ` [PATCH v2] " Vasiliy Kulikov
0 siblings, 1 reply; 6+ messages in thread
From: Patrick McHardy @ 2011-03-15 12:33 UTC (permalink / raw)
To: Changli Gao
Cc: Vasiliy Kulikov, linux-kernel, security, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
netfilter-devel, netfilter, coreteam, netdev
On 13.03.2011 15:00, Changli Gao wrote:
> On Fri, Mar 11, 2011 at 2:12 AM, Vasiliy Kulikov <segoon@openwall.com> wrote:
>> buffer string is copied from userspace. It is not checked whether it is
>> zero terminated. This may lead to overflow inside of simple_strtoul().
>>
>> It was introduced before the git epoch. Files "ipt_CLUSTERIP/*" are
>> root writable only by default, however, on some setups permissions might be
>> relaxed to e.g. network admin user.
>>
>> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
>> ---
>> Compile tested.
>>
>> net/ipv4/netfilter/ipt_CLUSTERIP.c | 1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c
>> index 403ca57..7aabf9a 100644
>> --- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
>> +++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
>> @@ -666,6 +666,7 @@ static ssize_t clusterip_proc_write(struct file *file, const char __user *input,
>>
>> if (copy_from_user(buffer, input, PROC_WRITELEN))
>
> I think size should be used instead of PROC_WRITELEN.
>
> if (size > PROC_WRITELEN)
> return -EIO;
> if (copy_from_user(buffer, input, size))
> return -EFAULT;
> buffer[size] = '\0';
I agree, otherwise we might have the situation that the userspace
copy is crossing page boundaries into unmapped memory.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] ipv4: netfilter: ipt_CLUSTERIP: fix buffer overflow
2011-03-15 12:33 ` Patrick McHardy
@ 2011-03-17 11:32 ` Vasiliy Kulikov
2011-03-17 12:15 ` Changli Gao
0 siblings, 1 reply; 6+ messages in thread
From: Vasiliy Kulikov @ 2011-03-17 11:32 UTC (permalink / raw)
To: Patrick McHardy
Cc: Changli Gao, linux-kernel, security, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
netfilter-devel, netfilter, coreteam, netdev
'buffer' string is copied from userspace. It is not checked whether it is
zero terminated. This may lead to overflow inside of simple_strtoul().
Changli Gao suggested to copy not more than user supplied 'size' bytes.
It was introduced before the git epoch. Files "ipt_CLUSTERIP/*" are
root writable only by default, however, on some setups permissions might be
relaxed to e.g. network admin user.
Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
net/ipv4/netfilter/ipt_CLUSTERIP.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c
index 1e26a48..af7dec6 100644
--- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
+++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
@@ -669,8 +669,11 @@ static ssize_t clusterip_proc_write(struct file *file, const char __user *input,
char buffer[PROC_WRITELEN+1];
unsigned long nodenum;
- if (copy_from_user(buffer, input, PROC_WRITELEN))
+ if (size > PROC_WRITELEN)
+ return -EIO;
+ if (copy_from_user(buffer, input, size))
return -EFAULT;
+ buffer[size] = 0;
if (*buffer == '+') {
nodenum = simple_strtoul(buffer+1, NULL, 10);
--
1.7.0.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] ipv4: netfilter: ipt_CLUSTERIP: fix buffer overflow
2011-03-17 11:32 ` [PATCH v2] " Vasiliy Kulikov
@ 2011-03-17 12:15 ` Changli Gao
2011-03-20 14:43 ` Patrick McHardy
0 siblings, 1 reply; 6+ messages in thread
From: Changli Gao @ 2011-03-17 12:15 UTC (permalink / raw)
To: Vasiliy Kulikov
Cc: Patrick McHardy, linux-kernel, security, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
netfilter-devel, netfilter, coreteam, netdev
On Thu, Mar 17, 2011 at 7:32 PM, Vasiliy Kulikov <segoon@openwall.com> wrote:
> 'buffer' string is copied from userspace. It is not checked whether it is
> zero terminated. This may lead to overflow inside of simple_strtoul().
> Changli Gao suggested to copy not more than user supplied 'size' bytes.
>
> It was introduced before the git epoch. Files "ipt_CLUSTERIP/*" are
> root writable only by default, however, on some setups permissions might be
> relaxed to e.g. network admin user.
>
> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
Acked-by: Changli Gao <xiaosuo@gmail.com>
--
Regards,
Changli Gao(xiaosuo@gmail.com)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ipv4: netfilter: ipt_CLUSTERIP: fix buffer overflow
2011-03-17 12:15 ` Changli Gao
@ 2011-03-20 14:43 ` Patrick McHardy
0 siblings, 0 replies; 6+ messages in thread
From: Patrick McHardy @ 2011-03-20 14:43 UTC (permalink / raw)
To: Changli Gao
Cc: Vasiliy Kulikov, linux-kernel, security, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
netfilter-devel, netfilter, coreteam, netdev
Am 17.03.2011 13:15, schrieb Changli Gao:
> On Thu, Mar 17, 2011 at 7:32 PM, Vasiliy Kulikov <segoon@openwall.com> wrote:
>> 'buffer' string is copied from userspace. It is not checked whether it is
>> zero terminated. This may lead to overflow inside of simple_strtoul().
>> Changli Gao suggested to copy not more than user supplied 'size' bytes.
>>
>> It was introduced before the git epoch. Files "ipt_CLUSTERIP/*" are
>> root writable only by default, however, on some setups permissions might be
>> relaxed to e.g. network admin user.
>>
>> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
> Acked-by: Changli Gao <xiaosuo@gmail.com>
>
>
Applied, thanks everyone.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-03-20 14:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-10 18:12 [PATCH] ipv4: netfilter: ipt_CLUSTERIP: fix buffer overflow Vasiliy Kulikov
2011-03-13 14:00 ` Changli Gao
2011-03-15 12:33 ` Patrick McHardy
2011-03-17 11:32 ` [PATCH v2] " Vasiliy Kulikov
2011-03-17 12:15 ` Changli Gao
2011-03-20 14:43 ` Patrick McHardy
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).