ip_queue, is this safe:

ip_queue, is this safe:

[Peteris Krumins]

Hello,

  I have written an application which uses ip_queue mechanism to quickly
  push (of course only some) packets from kernel space to user space.
  (The packets are queued in PREROUTING, mangle)
  The application is quite big and i bet i have made several mistakes,
  leaving unpleasent buffer overflows and something more.

  what i am interested in is if this is safe to write:

---
  [...]
  struct iphdr *ip;
  ip = (struct iphdr *)packet->payload;

  if (ip->protocol == IPPROTO_TCP) {
      struct tcphdr *tcp;

      tcp = (struct tcphdr *)(packet->payload + ip->ihl*4);
      [...]
  }
  [...]
---

  i am doubting:
  tcp = (struct tcphdr *)(packet->payload + ip->ihl*4);

  because ip->ihl could be faked, couldnt it? The packet would get queued
  and if ip->ihl is something faked, using the tcp pointer i'd get a
  segfault, wouldn't i?


P.Krumins

Re: ip_queue, is this safe:

[James Morris]

On Fri, 1 Aug 2003, Peteris Krumins wrote:

>   i am doubting:
>   tcp = (struct tcphdr *)(packet->payload + ip->ihl*4);

To start with, this pointer arithmetic is incorrect, you need something 
like:

	tcph = (void *) iph + iph->ihl * 4;

> 
>   because ip->ihl could be faked, couldnt it? The packet would get queued
>   and if ip->ihl is something faked, using the tcp pointer i'd get a
>   segfault, wouldn't i?

The kernel drops received packets with a too-large ihl before the
prerouting hook.  For locally generated packets of this type (e.g. root is
playing games with raw sockets), iptables 'accepts' them without 
traversing any more rules on the chain -- they won't make it to the 
QUEUE target.

You will however need to validate any transport+ header lengths against 
the length of the packet.


- James
-- 
James Morris
<jmorris@intercode.com.au>

Re[2]: ip_queue, is this safe:

[Peteris Krumins]

Friday, August 1, 2003, 6:25:18 PM, you wrote:

JM> On Fri, 1 Aug 2003, Peteris Krumins wrote:

>>   i am doubting:
>>   tcp = (struct tcphdr *)(packet->payload + ip->ihl*4);

JM> To start with, this pointer arithmetic is incorrect, you need something 
JM> like:

JM>         tcph = (void *) iph + iph->ihl * 4;

>> 
>>   because ip->ihl could be faked, couldnt it? The packet would get queued
>>   and if ip->ihl is something faked, using the tcp pointer i'd get a
>>   segfault, wouldn't i?

JM> The kernel drops received packets with a too-large ihl before the
JM> prerouting hook.  For locally generated packets of this type (e.g. root is
JM> playing games with raw sockets), iptables 'accepts' them without 
JM> traversing any more rules on the chain -- they won't make it to the 
JM> QUEUE target.

JM> You will however need to validate any transport+ header lengths against 
JM> the length of the packet.


Thanks for the answer!


P.Krumins