Re: [PATCH] icmp: Fixed bug in raw sockets causing incorrect ICMP SNMP counter values

From: Ben Cox
Date: Sun Oct 11 2015 - 18:17:09 EST


Forgive me for possibly being a little stupid here (This is my first
patch to Linux so I am slightly over my head)

Is this issue not addressed above the file where the following check is done?

if (iphlen > length)
goto error_free;



On Sun, Oct 11, 2015 at 11:10 PM, Eric Dumazet <eric.dumazet@xxxxxxxxx> wrote:
> On Sun, 2015-10-11 at 20:55 +0000, Ben Cartwright-Cox wrote:
>> Sending ICMP packets with raw sockets ends up in the SNMP counters
>> logging the type as the first byte of the IPv4 header rather than
>> the ICMP header (in nearly all cases this is seen as "OutType69".
>> This is fixed by adding the IP Header Length to the casting into
>> a icmphdr struct.
>>
>> Signed-off-by: Ben Cartwright-Cox <ben@xxxxxxxxxxxxx>
>> ---
>> net/ipv4/raw.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
>> index 561cd4b..1ad8bae 100644
>> --- a/net/ipv4/raw.c
>> +++ b/net/ipv4/raw.c
>> @@ -409,7 +409,7 @@ static int raw_send_hdrinc(struct sock *sk, struct flowi4 *fl4,
>> }
>> if (iph->protocol == IPPROTO_ICMP)
>> icmp_out_count(net, ((struct icmphdr *)
>> - skb_transport_header(skb))->type);
>> + skb_transport_header(skb) + iphlen)->type);
>>
>> err = NF_HOOK(NFPROTO_IPV4, NF_INET_LOCAL_OUT, sk, skb,
>> NULL, rt->dst.dev, dst_output_sk);
>
>
> Hmm... This seems to lack checks against a malicious user ?
>
> The only guarantee you have here is that iphlen < length.
>
> It is not enough.
>
> Make sure you do not access not initialized memory or even non existent
> one.
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/