| Message ID | 1333290170-26898-1-git-send-email-xiaosuo@gmail.com |
|---|---|
| State | Rejected |
| Headers | show |
On Sun, Apr 01, 2012 at 10:22:50PM +0800, Changli Gao wrote: > We should check the length of the data before dereferencing it when parsing > the TCP options. > > Signed-off-by: Changli Gao <xiaosuo@gmail.com> > --- > net/netfilter/nf_conntrack_proto_tcp.c | 4 ++++ > 1 file changed, 4 insertions(+) > diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c > index 361eade..9e446c5 100644 > --- a/net/netfilter/nf_conntrack_proto_tcp.c > +++ b/net/netfilter/nf_conntrack_proto_tcp.c > @@ -404,6 +404,8 @@ static void tcp_options(const struct sk_buff *skb, > length--; > continue; > default: > + if (length < 2) > + return; > opsize=*ptr++; > if (opsize < 2) /* "silly options" */ > return; length is always multiple of 4: int length = (tcph->doff*4) - sizeof(struct tcphdr); -- 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
On Sun, 2012-04-01 at 18:43 +0200, Pablo Neira Ayuso wrote: > On Sun, Apr 01, 2012 at 10:22:50PM +0800, Changli Gao wrote: > > We should check the length of the data before dereferencing it when parsing > > the TCP options. > > > > Signed-off-by: Changli Gao <xiaosuo@gmail.com> > > --- > > net/netfilter/nf_conntrack_proto_tcp.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c > > index 361eade..9e446c5 100644 > > --- a/net/netfilter/nf_conntrack_proto_tcp.c > > +++ b/net/netfilter/nf_conntrack_proto_tcp.c > > @@ -404,6 +404,8 @@ static void tcp_options(const struct sk_buff *skb, > > length--; > > continue; > > default: > > + if (length < 2) > > + return; > > opsize=*ptr++; > > if (opsize < 2) /* "silly options" */ > > return; > > length is always multiple of 4: > > int length = (tcph->doff*4) - sizeof(struct tcphdr); > -- initial value yes, but it can change in the loop. -- 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
On Sun, Apr 01, 2012 at 06:55:22PM +0200, Eric Dumazet wrote: > On Sun, 2012-04-01 at 18:43 +0200, Pablo Neira Ayuso wrote: > > On Sun, Apr 01, 2012 at 10:22:50PM +0800, Changli Gao wrote: > > > We should check the length of the data before dereferencing it when parsing > > > the TCP options. > > > > > > Signed-off-by: Changli Gao <xiaosuo@gmail.com> > > > --- > > > net/netfilter/nf_conntrack_proto_tcp.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c > > > index 361eade..9e446c5 100644 > > > --- a/net/netfilter/nf_conntrack_proto_tcp.c > > > +++ b/net/netfilter/nf_conntrack_proto_tcp.c > > > @@ -404,6 +404,8 @@ static void tcp_options(const struct sk_buff *skb, > > > length--; > > > continue; > > > default: > > > + if (length < 2) > > > + return; > > > opsize=*ptr++; > > > if (opsize < 2) /* "silly options" */ > > > return; > > > > length is always multiple of 4: > > > > int length = (tcph->doff*4) - sizeof(struct tcphdr); > > -- > > initial value yes, but it can change in the loop. Indeed, then I think we need a similar patch for tcp_parse_options() in net/ipv4/tcp_input.c -- 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
On Mon, Apr 2, 2012 at 1:02 AM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > Indeed, then I think we need a similar patch for tcp_parse_options() > in net/ipv4/tcp_input.c I'll send a separated patch for it. Thanks.
diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c index 361eade..9e446c5 100644 --- a/net/netfilter/nf_conntrack_proto_tcp.c +++ b/net/netfilter/nf_conntrack_proto_tcp.c @@ -404,6 +404,8 @@ static void tcp_options(const struct sk_buff *skb, length--; continue; default: + if (length < 2) + return; opsize=*ptr++; if (opsize < 2) /* "silly options" */ return; @@ -464,6 +466,8 @@ static void tcp_sack(const struct sk_buff *skb, unsigned int dataoff, length--; continue; default: + if (length < 2) + return; opsize = *ptr++; if (opsize < 2) /* "silly options" */ return;
We should check the length of the data before dereferencing it when parsing the TCP options. Signed-off-by: Changli Gao <xiaosuo@gmail.com> --- net/netfilter/nf_conntrack_proto_tcp.c | 4 ++++ 1 file changed, 4 insertions(+) -- 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