Skip to content

Commit

Permalink
netfilter: x_tables: don't move to non-existent next rule
Browse files Browse the repository at this point in the history
commit f24e230d257af1ad7476c6e81a8dc3127a74204e 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.

CVE-2016-3134

Change-Id: Id7a9c52147dd20ab7f789e98b6f320a1534c2a94
Reported-by: Ben Hawkes <[email protected]>
Signed-off-by: Florian Westphal <[email protected]>
Signed-off-by: Pablo Neira Ayuso <[email protected]>
Signed-off-by: Chas Williams <[email protected]>
Signed-off-by: Willy Tarreau <[email protected]>
  • Loading branch information
Florian Westphal authored and npjohnson committed Mar 11, 2021
1 parent 97a0389 commit ce15423
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 3 deletions.
8 changes: 5 additions & 3 deletions net/ipv4/netfilter/arp_tables.c
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,8 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
size = e->next_offset;
e = (struct arpt_entry *)
(entry0 + pos + size);
if (pos + size >= newinfo->size)
return 0;
e->counters.pcnt = pos;
pos += size;
} else {
Expand All @@ -457,6 +459,8 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
} else {
/* ... this is a fallthru */
newpos = pos + e->next_offset;
if (newpos >= newinfo->size)
return 0;
}
e = (struct arpt_entry *)
(entry0 + newpos);
Expand Down Expand Up @@ -680,10 +684,8 @@ static int translate_table(struct xt_table_info *newinfo, void *entry0,
}
}

if (!mark_source_chains(newinfo, repl->valid_hooks, entry0)) {
duprintf("Looping hook\n");
if (!mark_source_chains(newinfo, repl->valid_hooks, entry0))
return -ELOOP;
}

/* Finally, each sanity check must pass */
i = 0;
Expand Down
4 changes: 4 additions & 0 deletions net/ipv4/netfilter/ip_tables.c
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
size = e->next_offset;
e = (struct ipt_entry *)
(entry0 + pos + size);
if (pos + size >= newinfo->size)
return 0;
e->counters.pcnt = pos;
pos += size;
} else {
Expand All @@ -536,6 +538,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
} else {
/* ... this is a fallthru */
newpos = pos + e->next_offset;
if (newpos >= newinfo->size)
return 0;
}
e = (struct ipt_entry *)
(entry0 + newpos);
Expand Down
4 changes: 4 additions & 0 deletions net/ipv6/netfilter/ip6_tables.c
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
size = e->next_offset;
e = (struct ip6t_entry *)
(entry0 + pos + size);
if (pos + size >= newinfo->size)
return 0;
e->counters.pcnt = pos;
pos += size;
} else {
Expand All @@ -548,6 +550,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
} else {
/* ... this is a fallthru */
newpos = pos + e->next_offset;
if (newpos >= newinfo->size)
return 0;
}
e = (struct ip6t_entry *)
(entry0 + newpos);
Expand Down

0 comments on commit ce15423

Please sign in to comment.