* [PATCH 2/8] Add sanity checkings for ICMP
@ 2005-12-05 11:20 Pablo Neira Ayuso
2005-12-09 2:40 ` Yasuyuki KOZAKAI
[not found] ` <200512090240.jB92egAp002675@toshiba.co.jp>
0 siblings, 2 replies; 3+ messages in thread
From: Pablo Neira Ayuso @ 2005-12-05 11:20 UTC (permalink / raw)
To: Netfilter Development Mailinglist; +Cc: Patrick McHardy
[-- Attachment #1: Type: text/plain, Size: 146 bytes --]
--
The dawn of the fourth age of Linux firewalling is coming; a time of
great struggle and heroic deeds -- J.Kadlecsik got inspired by J.Morris
[-- Attachment #2: 11.patch --]
[-- Type: text/plain, Size: 1003 bytes --]
Add proper checkings to avoid possible malformed ICMP conntracks. And return
to userspace -EINVAL in case of error.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Index: netfilter-2.6.14.git/net/ipv4/netfilter/ip_conntrack_proto_icmp.c
===================================================================
--- netfilter-2.6.14.git.orig/net/ipv4/netfilter/ip_conntrack_proto_icmp.c 2005-12-04 03:35:50.000000000 +0100
+++ netfilter-2.6.14.git/net/ipv4/netfilter/ip_conntrack_proto_icmp.c 2005-12-04 03:37:24.000000000 +0100
@@ -288,10 +288,14 @@ nfattr_failure:
static int icmp_nfattr_to_tuple(struct nfattr *tb[],
struct ip_conntrack_tuple *tuple)
{
+ if (tuple->dst.u.icmp.type >= sizeof(valid_new)
+ || !valid_new[tuple->dst.u.icmp.type])
+ return -EINVAL;
+
if (!tb[CTA_PROTO_ICMP_TYPE-1]
|| !tb[CTA_PROTO_ICMP_CODE-1]
|| !tb[CTA_PROTO_ICMP_ID-1])
- return -1;
+ return -EINVAL;
tuple->dst.u.icmp.type =
*(u_int8_t *)NFA_DATA(tb[CTA_PROTO_ICMP_TYPE-1]);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 2/8] Add sanity checkings for ICMP
2005-12-05 11:20 [PATCH 2/8] Add sanity checkings for ICMP Pablo Neira Ayuso
@ 2005-12-09 2:40 ` Yasuyuki KOZAKAI
[not found] ` <200512090240.jB92egAp002675@toshiba.co.jp>
1 sibling, 0 replies; 3+ messages in thread
From: Yasuyuki KOZAKAI @ 2005-12-09 2:40 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, kaber
[-- Attachment #1: Type: Text/Plain, Size: 1466 bytes --]
Hi, Pablo,
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Mon, 05 Dec 2005 12:20:59 +0100
> Add proper checkings to avoid possible malformed ICMP conntracks. And return
> to userspace -EINVAL in case of error.
>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
>
> Index: netfilter-2.6.14.git/net/ipv4/netfilter/ip_conntrack_proto_icmp.c
> ===================================================================
> --- netfilter-2.6.14.git.orig/net/ipv4/netfilter/ip_conntrack_proto_icmp.c 2005-12-04 03:35:50.000000000 +0100
> +++ netfilter-2.6.14.git/net/ipv4/netfilter/ip_conntrack_proto_icmp.c 2005-12-04 03:37:24.000000000 +0100
> @@ -288,10 +288,14 @@ nfattr_failure:
> static int icmp_nfattr_to_tuple(struct nfattr *tb[],
> struct ip_conntrack_tuple *tuple)
> {
> + if (tuple->dst.u.icmp.type >= sizeof(valid_new)
> + || !valid_new[tuple->dst.u.icmp.type])
> + return -EINVAL;
> +
> if (!tb[CTA_PROTO_ICMP_TYPE-1]
> || !tb[CTA_PROTO_ICMP_CODE-1]
> || !tb[CTA_PROTO_ICMP_ID-1])
> - return -1;
> + return -EINVAL;
>
> tuple->dst.u.icmp.type =
> *(u_int8_t *)NFA_DATA(tb[CTA_PROTO_ICMP_TYPE-1]);
This checks ICMP type in tuple before putting it.
And it's better to use invmap instead of valid_new to allow Echo reply and
so on.
How about attached patch instead ?
# Please note that attached patch doesn't include your 1st patch.
# You can just replace your 2nd patch with it.
Regards,
-- Yasuyuki Kozakai
[-- Attachment #2: 04-ipctnetlink-check-icmp.patch --]
[-- Type: Text/Plain, Size: 3251 bytes --]
[NETFILTER] ipctnetlink: Add sanity checkings for ICMP
Signed-off-by: Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp>
---
commit 2cae73ad22d11072b7787b70f5d38f443d110491
tree c6e05f2f2407f4c21ea48b990816bfce50c209e4
parent b01d2778bc7ca3049fed4eb4b216e063faa52477
author Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp> Fri, 09 Dec 2005 02:01:49 +0900
committer Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp> Fri, 09 Dec 2005 02:01:49 +0900
net/ipv4/netfilter/ip_conntrack_proto_icmp.c | 43 +++++++++++++++-----------
1 files changed, 24 insertions(+), 19 deletions(-)
diff --git a/net/ipv4/netfilter/ip_conntrack_proto_icmp.c b/net/ipv4/netfilter/ip_conntrack_proto_icmp.c
index 19cc550..30fc21d 100644
--- a/net/ipv4/netfilter/ip_conntrack_proto_icmp.c
+++ b/net/ipv4/netfilter/ip_conntrack_proto_icmp.c
@@ -47,20 +47,21 @@ static int icmp_pkt_to_tuple(const struc
return 1;
}
+/* Add 1; spaces filled with 0. */
+static const u_int8_t invmap[] = {
+ [ICMP_ECHO] = ICMP_ECHOREPLY + 1,
+ [ICMP_ECHOREPLY] = ICMP_ECHO + 1,
+ [ICMP_TIMESTAMP] = ICMP_TIMESTAMPREPLY + 1,
+ [ICMP_TIMESTAMPREPLY] = ICMP_TIMESTAMP + 1,
+ [ICMP_INFO_REQUEST] = ICMP_INFO_REPLY + 1,
+ [ICMP_INFO_REPLY] = ICMP_INFO_REQUEST + 1,
+ [ICMP_ADDRESS] = ICMP_ADDRESSREPLY + 1,
+ [ICMP_ADDRESSREPLY] = ICMP_ADDRESS + 1
+};
+
static int icmp_invert_tuple(struct ip_conntrack_tuple *tuple,
const struct ip_conntrack_tuple *orig)
{
- /* Add 1; spaces filled with 0. */
- static const u_int8_t invmap[]
- = { [ICMP_ECHO] = ICMP_ECHOREPLY + 1,
- [ICMP_ECHOREPLY] = ICMP_ECHO + 1,
- [ICMP_TIMESTAMP] = ICMP_TIMESTAMPREPLY + 1,
- [ICMP_TIMESTAMPREPLY] = ICMP_TIMESTAMP + 1,
- [ICMP_INFO_REQUEST] = ICMP_INFO_REPLY + 1,
- [ICMP_INFO_REPLY] = ICMP_INFO_REQUEST + 1,
- [ICMP_ADDRESS] = ICMP_ADDRESSREPLY + 1,
- [ICMP_ADDRESSREPLY] = ICMP_ADDRESS + 1};
-
if (orig->dst.u.icmp.type >= sizeof(invmap)
|| !invmap[orig->dst.u.icmp.type])
return 0;
@@ -110,17 +111,17 @@ static int icmp_packet(struct ip_conntra
return NF_ACCEPT;
}
-static const u_int8_t valid_new[] = {
- [ICMP_ECHO] = 1,
- [ICMP_TIMESTAMP] = 1,
- [ICMP_INFO_REQUEST] = 1,
- [ICMP_ADDRESS] = 1
-};
-
/* Called when a new connection for this protocol found. */
static int icmp_new(struct ip_conntrack *conntrack,
const struct sk_buff *skb)
{
+ static const u_int8_t valid_new[] = {
+ [ICMP_ECHO] = 1,
+ [ICMP_TIMESTAMP] = 1,
+ [ICMP_INFO_REQUEST] = 1,
+ [ICMP_ADDRESS] = 1
+ };
+
if (conntrack->tuplehash[0].tuple.dst.u.icmp.type >= sizeof(valid_new)
|| !valid_new[conntrack->tuplehash[0].tuple.dst.u.icmp.type]) {
/* Can't create a new ICMP `conn' with this. */
@@ -291,7 +292,7 @@ static int icmp_nfattr_to_tuple(struct n
if (!tb[CTA_PROTO_ICMP_TYPE-1]
|| !tb[CTA_PROTO_ICMP_CODE-1]
|| !tb[CTA_PROTO_ICMP_ID-1])
- return -1;
+ return -EINVAL;
tuple->dst.u.icmp.type =
*(u_int8_t *)NFA_DATA(tb[CTA_PROTO_ICMP_TYPE-1]);
@@ -300,6 +301,10 @@ static int icmp_nfattr_to_tuple(struct n
tuple->src.u.icmp.id =
*(u_int16_t *)NFA_DATA(tb[CTA_PROTO_ICMP_ID-1]);
+ if (tuple->dst.u.icmp.type >= sizeof(invmap)
+ || !invmap[tuple->dst.u.icmp.type])
+ return -EINVAL;
+
return 0;
}
#endif
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 2/8] Add sanity checkings for ICMP
[not found] ` <200512090240.jB92egAp002675@toshiba.co.jp>
@ 2005-12-15 0:49 ` Patrick McHardy
0 siblings, 0 replies; 3+ messages in thread
From: Patrick McHardy @ 2005-12-15 0:49 UTC (permalink / raw)
To: Yasuyuki KOZAKAI; +Cc: netfilter-devel, pablo
Yasuyuki KOZAKAI wrote:
> This checks ICMP type in tuple before putting it.
> And it's better to use invmap instead of valid_new to allow Echo reply and
> so on.
>
> How about attached patch instead ?
>
> [NETFILTER] ipctnetlink: Add sanity checkings for ICMP
Applied to my 2.6.16 queue, thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2005-12-15 0:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-12-05 11:20 [PATCH 2/8] Add sanity checkings for ICMP Pablo Neira Ayuso
2005-12-09 2:40 ` Yasuyuki KOZAKAI
[not found] ` <200512090240.jB92egAp002675@toshiba.co.jp>
2005-12-15 0:49 ` Patrick McHardy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.