[patch 05/23] NETFILTER: Missed and reordered checks in {arp,ip,ip6}_tables
-stable review patch. If anyone has any objections, please let us know.
--
From: Patrick McHardy <[EMAIL PROTECTED]>
Backport fix for missing ruleset validation in {arp,ip,ip6}_tables
and a fix on top which fixes a regression in the first patch.
There is a number of issues in parsing user-provided table in
translate_table(). Malicious user with CAP_NET_ADMIN may crash system by
passing special-crafted table to the *_tables.
The first issue is that mark_source_chains() function is called before entry
content checks. In case of standard target, mark_source_chains() function
uses t->verdict field in order to determine new position. But the check, that
this field leads no further, than the table end, is in check_entry(), which
is called later, than mark_source_chains().
The second issue, that there is no check that target_offset points inside
entry. If so, *_ITERATE_MATCH macro will follow further, than the entry
ends. As a result, we'll have oops or memory disclosure.
And the third issue, that there is no check that the target is completely
inside entry. Results are the same, as in previous issue.
Upstream commit 590bdf7fd2292b47c428111cb1360e312eff207e introduced a
regression in match/target hook validation. mark_source_chains builds
a bitmask for each rule representing the hooks it can be reached from,
which is then used by the matches and targets to make sure they are
only called from valid hooks. The patch moved the match/target specific
validation before the mark_source_chains call, at which point the mask
is always zero.
This patch returns back to the old order and moves the standard checks
to mark_source_chains. This allows to get rid of a special case for
standard targets as a nice side-effect.
Signed-off-by: Patrick McHardy <[EMAIL PROTECTED]>
Signed-off-by: Chris Wright <[EMAIL PROTECTED]>
---
commit 1cfcb663c5a6d8b4b1172ff481af1b597bc8b54e
tree 61c5b135ee292681f38945a3549cb9005aec1d7c
parent b2ab160e1a3a1eb3fcc80132d8d7db5687a7a113
author Patrick McHardy <[EMAIL PROTECTED]> Tue, 21 Nov 2006 11:17:03 +0100
committer Patrick McHardy <[EMAIL PROTECTED]> Tue, 21 Nov 2006 11:24:51 +0100
net/ipv4/netfilter/arp_tables.c | 37 +-
net/ipv4/netfilter/ip_tables.c | 65 +++-
net/ipv6/netfilter/ip6_tables.c | 53 ++--
3 files changed, 77 insertions(+), 78 deletions(-)
--- linux-2.6.18.4.orig/net/ipv4/netfilter/arp_tables.c
+++ linux-2.6.18.4/net/ipv4/netfilter/arp_tables.c
@@ -380,6 +380,13 @@ static int mark_source_chains(struct xt_
&& unconditional(>arp)) {
unsigned int oldpos, size;
+ if (t->verdict < -NF_MAX_VERDICT - 1) {
+ duprintf("mark_source_chains: bad "
+ "negative verdict (%i)\n",
+ t->verdict);
+ return 0;
+ }
+
/* Return: backtrack through the last
* big jump.
*/
@@ -409,6 +416,14 @@ static int mark_source_chains(struct xt_
if (strcmp(t->target.u.user.name,
ARPT_STANDARD_TARGET) == 0
&& newpos >= 0) {
+ if (newpos > newinfo->size -
+ sizeof(struct arpt_entry)) {
+ duprintf("mark_source_chains: "
+ "bad verdict (%i)\n",
+ newpos);
+ return 0;
+ }
+
/* This a jump; chase it. */
duprintf("Jump rule %u -> %u\n",
pos, newpos);
@@ -431,8 +446,6 @@ static int mark_source_chains(struct xt_
static inline int standard_check(const struct arpt_entry_target *t,
unsigned int max_offset)
{
- struct arpt_standard_target *targ = (void *)t;
-
/* Check standard info. */
if (t->u.target_size
!= ARPT_ALIGN(sizeof(struct arpt_standard_target))) {
@@ -442,18 +455,6 @@ static inline int standard_check(const s
return 0;
}
- if (targ->verdict >= 0
- && targ->verdict > max_offset - sizeof(struct arpt_entry)) {
- duprintf("arpt_standard_check: bad verdict (%i)\n",
-targ->verdict);
- return 0;
- }
-
- if (targ->verdict < -NF_MAX_VERDICT - 1) {
-
[patch 05/23] NETFILTER: Missed and reordered checks in {arp,ip,ip6}_tables
-stable review patch. If anyone has any objections, please let us know.
--
From: Patrick McHardy [EMAIL PROTECTED]
Backport fix for missing ruleset validation in {arp,ip,ip6}_tables
and a fix on top which fixes a regression in the first patch.
There is a number of issues in parsing user-provided table in
translate_table(). Malicious user with CAP_NET_ADMIN may crash system by
passing special-crafted table to the *_tables.
The first issue is that mark_source_chains() function is called before entry
content checks. In case of standard target, mark_source_chains() function
uses t-verdict field in order to determine new position. But the check, that
this field leads no further, than the table end, is in check_entry(), which
is called later, than mark_source_chains().
The second issue, that there is no check that target_offset points inside
entry. If so, *_ITERATE_MATCH macro will follow further, than the entry
ends. As a result, we'll have oops or memory disclosure.
And the third issue, that there is no check that the target is completely
inside entry. Results are the same, as in previous issue.
Upstream commit 590bdf7fd2292b47c428111cb1360e312eff207e introduced a
regression in match/target hook validation. mark_source_chains builds
a bitmask for each rule representing the hooks it can be reached from,
which is then used by the matches and targets to make sure they are
only called from valid hooks. The patch moved the match/target specific
validation before the mark_source_chains call, at which point the mask
is always zero.
This patch returns back to the old order and moves the standard checks
to mark_source_chains. This allows to get rid of a special case for
standard targets as a nice side-effect.
Signed-off-by: Patrick McHardy [EMAIL PROTECTED]
Signed-off-by: Chris Wright [EMAIL PROTECTED]
---
commit 1cfcb663c5a6d8b4b1172ff481af1b597bc8b54e
tree 61c5b135ee292681f38945a3549cb9005aec1d7c
parent b2ab160e1a3a1eb3fcc80132d8d7db5687a7a113
author Patrick McHardy [EMAIL PROTECTED] Tue, 21 Nov 2006 11:17:03 +0100
committer Patrick McHardy [EMAIL PROTECTED] Tue, 21 Nov 2006 11:24:51 +0100
net/ipv4/netfilter/arp_tables.c | 37 +-
net/ipv4/netfilter/ip_tables.c | 65 +++-
net/ipv6/netfilter/ip6_tables.c | 53 ++--
3 files changed, 77 insertions(+), 78 deletions(-)
--- linux-2.6.18.4.orig/net/ipv4/netfilter/arp_tables.c
+++ linux-2.6.18.4/net/ipv4/netfilter/arp_tables.c
@@ -380,6 +380,13 @@ static int mark_source_chains(struct xt_
unconditional(e-arp)) {
unsigned int oldpos, size;
+ if (t-verdict -NF_MAX_VERDICT - 1) {
+ duprintf(mark_source_chains: bad
+ negative verdict (%i)\n,
+ t-verdict);
+ return 0;
+ }
+
/* Return: backtrack through the last
* big jump.
*/
@@ -409,6 +416,14 @@ static int mark_source_chains(struct xt_
if (strcmp(t-target.u.user.name,
ARPT_STANDARD_TARGET) == 0
newpos = 0) {
+ if (newpos newinfo-size -
+ sizeof(struct arpt_entry)) {
+ duprintf(mark_source_chains:
+ bad verdict (%i)\n,
+ newpos);
+ return 0;
+ }
+
/* This a jump; chase it. */
duprintf(Jump rule %u - %u\n,
pos, newpos);
@@ -431,8 +446,6 @@ static int mark_source_chains(struct xt_
static inline int standard_check(const struct arpt_entry_target *t,
unsigned int max_offset)
{
- struct arpt_standard_target *targ = (void *)t;
-
/* Check standard info. */
if (t-u.target_size
!= ARPT_ALIGN(sizeof(struct arpt_standard_target))) {
@@ -442,18 +455,6 @@ static inline int standard_check(const s
return 0;
}
- if (targ-verdict = 0
-targ-verdict max_offset - sizeof(struct arpt_entry)) {
- duprintf(arpt_standard_check: bad verdict (%i)\n,
-targ-verdict);
- return 0;
- }
-
- if (targ-verdict -NF_MAX_VERDICT - 1) {
- duprintf(arpt_standard_check: bad negative verdict (%i)\n,
-
