Skip to content

Commit d6f7cd1

Browse files
Florian Westphalgregkh
authored andcommitted
netfilter: x_tables: don't move to non-existent next rule
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. Reported-by: Ben Hawkes <hawkes@google.com> Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 0d6ad54 commit d6f7cd1

3 files changed

Lines changed: 13 additions & 3 deletions

File tree

net/ipv4/netfilter/arp_tables.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,8 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
439439
size = e->next_offset;
440440
e = (struct arpt_entry *)
441441
(entry0 + pos + size);
442+
if (pos + size >= newinfo->size)
443+
return 0;
442444
e->counters.pcnt = pos;
443445
pos += size;
444446
} else {
@@ -461,6 +463,8 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
461463
} else {
462464
/* ... this is a fallthru */
463465
newpos = pos + e->next_offset;
466+
if (newpos >= newinfo->size)
467+
return 0;
464468
}
465469
e = (struct arpt_entry *)
466470
(entry0 + newpos);
@@ -691,10 +695,8 @@ static int translate_table(struct xt_table_info *newinfo, void *entry0,
691695
}
692696
}
693697

694-
if (!mark_source_chains(newinfo, repl->valid_hooks, entry0)) {
695-
duprintf("Looping hook\n");
698+
if (!mark_source_chains(newinfo, repl->valid_hooks, entry0))
696699
return -ELOOP;
697-
}
698700

699701
/* Finally, each sanity check must pass */
700702
i = 0;

net/ipv4/netfilter/ip_tables.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
520520
size = e->next_offset;
521521
e = (struct ipt_entry *)
522522
(entry0 + pos + size);
523+
if (pos + size >= newinfo->size)
524+
return 0;
523525
e->counters.pcnt = pos;
524526
pos += size;
525527
} else {
@@ -541,6 +543,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
541543
} else {
542544
/* ... this is a fallthru */
543545
newpos = pos + e->next_offset;
546+
if (newpos >= newinfo->size)
547+
return 0;
544548
}
545549
e = (struct ipt_entry *)
546550
(entry0 + newpos);

net/ipv6/netfilter/ip6_tables.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
532532
size = e->next_offset;
533533
e = (struct ip6t_entry *)
534534
(entry0 + pos + size);
535+
if (pos + size >= newinfo->size)
536+
return 0;
535537
e->counters.pcnt = pos;
536538
pos += size;
537539
} else {
@@ -553,6 +555,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
553555
} else {
554556
/* ... this is a fallthru */
555557
newpos = pos + e->next_offset;
558+
if (newpos >= newinfo->size)
559+
return 0;
556560
}
557561
e = (struct ip6t_entry *)
558562
(entry0 + newpos);

0 commit comments

Comments
 (0)