Re: [PATCH] ipv4: netfilter: ipt_CLUSTERIP: fix buffer overflow

[Patrick McHardy <kaber@trash.net>]

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.