Commit 3d2d4415 authored by Florian Westphal's avatar Florian Westphal Committed by Ben Hutchings

netfilter: x_tables: don't move to non-existent next rule

commit f24e230d upstream.

Ben Hawkes says:

 In the mark_source_chains function (net/ipv4/netfilter/ip_tables.c) it
 is possible for a user-supplied ipt_entry structure to have a large
 next_offset field. This field is not bounds checked prior to writing a
 counter value at the supplied offset.

Base chains enforce absolute verdict.

User defined chains are supposed to end with an unconditional return,
xtables userspace adds them automatically.

But if such return is missing we will move to non-existent next rule.
Reported-by: default avatarBen Hawkes <hawkes@google.com>
Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: default avatarBen Hutchings <ben@decadent.org.uk>
parent 72aaf646
...@@ -435,6 +435,8 @@ static int mark_source_chains(const struct xt_table_info *newinfo, ...@@ -435,6 +435,8 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
size = e->next_offset; size = e->next_offset;
e = (struct arpt_entry *) e = (struct arpt_entry *)
(entry0 + pos + size); (entry0 + pos + size);
if (pos + size >= newinfo->size)
return 0;
e->counters.pcnt = pos; e->counters.pcnt = pos;
pos += size; pos += size;
} else { } else {
...@@ -457,6 +459,8 @@ static int mark_source_chains(const struct xt_table_info *newinfo, ...@@ -457,6 +459,8 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
} else { } else {
/* ... this is a fallthru */ /* ... this is a fallthru */
newpos = pos + e->next_offset; newpos = pos + e->next_offset;
if (newpos >= newinfo->size)
return 0;
} }
e = (struct arpt_entry *) e = (struct arpt_entry *)
(entry0 + newpos); (entry0 + newpos);
...@@ -680,10 +684,8 @@ static int translate_table(struct xt_table_info *newinfo, void *entry0, ...@@ -680,10 +684,8 @@ static int translate_table(struct xt_table_info *newinfo, void *entry0,
} }
} }
if (!mark_source_chains(newinfo, repl->valid_hooks, entry0)) { if (!mark_source_chains(newinfo, repl->valid_hooks, entry0))
duprintf("Looping hook\n");
return -ELOOP; return -ELOOP;
}
/* Finally, each sanity check must pass */ /* Finally, each sanity check must pass */
i = 0; i = 0;
......
...@@ -516,6 +516,8 @@ mark_source_chains(const struct xt_table_info *newinfo, ...@@ -516,6 +516,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
size = e->next_offset; size = e->next_offset;
e = (struct ipt_entry *) e = (struct ipt_entry *)
(entry0 + pos + size); (entry0 + pos + size);
if (pos + size >= newinfo->size)
return 0;
e->counters.pcnt = pos; e->counters.pcnt = pos;
pos += size; pos += size;
} else { } else {
...@@ -537,6 +539,8 @@ mark_source_chains(const struct xt_table_info *newinfo, ...@@ -537,6 +539,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
} else { } else {
/* ... this is a fallthru */ /* ... this is a fallthru */
newpos = pos + e->next_offset; newpos = pos + e->next_offset;
if (newpos >= newinfo->size)
return 0;
} }
e = (struct ipt_entry *) e = (struct ipt_entry *)
(entry0 + newpos); (entry0 + newpos);
......
...@@ -526,6 +526,8 @@ mark_source_chains(const struct xt_table_info *newinfo, ...@@ -526,6 +526,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
size = e->next_offset; size = e->next_offset;
e = (struct ip6t_entry *) e = (struct ip6t_entry *)
(entry0 + pos + size); (entry0 + pos + size);
if (pos + size >= newinfo->size)
return 0;
e->counters.pcnt = pos; e->counters.pcnt = pos;
pos += size; pos += size;
} else { } else {
...@@ -547,6 +549,8 @@ mark_source_chains(const struct xt_table_info *newinfo, ...@@ -547,6 +549,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
} else { } else {
/* ... this is a fallthru */ /* ... this is a fallthru */
newpos = pos + e->next_offset; newpos = pos + e->next_offset;
if (newpos >= newinfo->size)
return 0;
} }
e = (struct ip6t_entry *) e = (struct ip6t_entry *)
(entry0 + newpos); (entry0 + newpos);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment