netfilter: synproxy: fix BUG_ON triggered by corrupt TCP packets
TCP packets hitting the SYN proxy through the SYNPROXY target are not validated by TCP conntrack. When th->doff is below 5, an underflow happens when calculating the options length, causing skb_header_pointer() to return NULL and triggering the BUG_ON(). Handle this case gracefully by checking for NULL instead of using BUG_ON(). Reported-by: Martin Topholm <mph@one.com> Tested-by: Martin Topholm <mph@one.com> Signed-off-by: Patrick McHardy <kaber@trash.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
This commit is contained in:
committed by
Pablo Neira Ayuso
parent
d1ee4fea0b
commit
f4a87e7bd2
@@ -56,7 +56,7 @@ struct synproxy_options {
|
|||||||
|
|
||||||
struct tcphdr;
|
struct tcphdr;
|
||||||
struct xt_synproxy_info;
|
struct xt_synproxy_info;
|
||||||
extern void synproxy_parse_options(const struct sk_buff *skb, unsigned int doff,
|
extern bool synproxy_parse_options(const struct sk_buff *skb, unsigned int doff,
|
||||||
const struct tcphdr *th,
|
const struct tcphdr *th,
|
||||||
struct synproxy_options *opts);
|
struct synproxy_options *opts);
|
||||||
extern unsigned int synproxy_options_size(const struct synproxy_options *opts);
|
extern unsigned int synproxy_options_size(const struct synproxy_options *opts);
|
||||||
|
@@ -267,7 +267,8 @@ synproxy_tg4(struct sk_buff *skb, const struct xt_action_param *par)
|
|||||||
if (th == NULL)
|
if (th == NULL)
|
||||||
return NF_DROP;
|
return NF_DROP;
|
||||||
|
|
||||||
synproxy_parse_options(skb, par->thoff, th, &opts);
|
if (!synproxy_parse_options(skb, par->thoff, th, &opts))
|
||||||
|
return NF_DROP;
|
||||||
|
|
||||||
if (th->syn && !(th->ack || th->fin || th->rst)) {
|
if (th->syn && !(th->ack || th->fin || th->rst)) {
|
||||||
/* Initial SYN from client */
|
/* Initial SYN from client */
|
||||||
@@ -350,7 +351,8 @@ static unsigned int ipv4_synproxy_hook(unsigned int hooknum,
|
|||||||
|
|
||||||
/* fall through */
|
/* fall through */
|
||||||
case TCP_CONNTRACK_SYN_SENT:
|
case TCP_CONNTRACK_SYN_SENT:
|
||||||
synproxy_parse_options(skb, thoff, th, &opts);
|
if (!synproxy_parse_options(skb, thoff, th, &opts))
|
||||||
|
return NF_DROP;
|
||||||
|
|
||||||
if (!th->syn && th->ack &&
|
if (!th->syn && th->ack &&
|
||||||
CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL) {
|
CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL) {
|
||||||
@@ -373,7 +375,9 @@ static unsigned int ipv4_synproxy_hook(unsigned int hooknum,
|
|||||||
if (!th->syn || !th->ack)
|
if (!th->syn || !th->ack)
|
||||||
break;
|
break;
|
||||||
|
|
||||||
synproxy_parse_options(skb, thoff, th, &opts);
|
if (!synproxy_parse_options(skb, thoff, th, &opts))
|
||||||
|
return NF_DROP;
|
||||||
|
|
||||||
if (opts.options & XT_SYNPROXY_OPT_TIMESTAMP)
|
if (opts.options & XT_SYNPROXY_OPT_TIMESTAMP)
|
||||||
synproxy->tsoff = opts.tsval - synproxy->its;
|
synproxy->tsoff = opts.tsval - synproxy->its;
|
||||||
|
|
||||||
|
@@ -282,7 +282,8 @@ synproxy_tg6(struct sk_buff *skb, const struct xt_action_param *par)
|
|||||||
if (th == NULL)
|
if (th == NULL)
|
||||||
return NF_DROP;
|
return NF_DROP;
|
||||||
|
|
||||||
synproxy_parse_options(skb, par->thoff, th, &opts);
|
if (!synproxy_parse_options(skb, par->thoff, th, &opts))
|
||||||
|
return NF_DROP;
|
||||||
|
|
||||||
if (th->syn && !(th->ack || th->fin || th->rst)) {
|
if (th->syn && !(th->ack || th->fin || th->rst)) {
|
||||||
/* Initial SYN from client */
|
/* Initial SYN from client */
|
||||||
@@ -372,7 +373,8 @@ static unsigned int ipv6_synproxy_hook(unsigned int hooknum,
|
|||||||
|
|
||||||
/* fall through */
|
/* fall through */
|
||||||
case TCP_CONNTRACK_SYN_SENT:
|
case TCP_CONNTRACK_SYN_SENT:
|
||||||
synproxy_parse_options(skb, thoff, th, &opts);
|
if (!synproxy_parse_options(skb, thoff, th, &opts))
|
||||||
|
return NF_DROP;
|
||||||
|
|
||||||
if (!th->syn && th->ack &&
|
if (!th->syn && th->ack &&
|
||||||
CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL) {
|
CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL) {
|
||||||
@@ -395,7 +397,9 @@ static unsigned int ipv6_synproxy_hook(unsigned int hooknum,
|
|||||||
if (!th->syn || !th->ack)
|
if (!th->syn || !th->ack)
|
||||||
break;
|
break;
|
||||||
|
|
||||||
synproxy_parse_options(skb, thoff, th, &opts);
|
if (!synproxy_parse_options(skb, thoff, th, &opts))
|
||||||
|
return NF_DROP;
|
||||||
|
|
||||||
if (opts.options & XT_SYNPROXY_OPT_TIMESTAMP)
|
if (opts.options & XT_SYNPROXY_OPT_TIMESTAMP)
|
||||||
synproxy->tsoff = opts.tsval - synproxy->its;
|
synproxy->tsoff = opts.tsval - synproxy->its;
|
||||||
|
|
||||||
|
@@ -24,7 +24,7 @@
|
|||||||
int synproxy_net_id;
|
int synproxy_net_id;
|
||||||
EXPORT_SYMBOL_GPL(synproxy_net_id);
|
EXPORT_SYMBOL_GPL(synproxy_net_id);
|
||||||
|
|
||||||
void
|
bool
|
||||||
synproxy_parse_options(const struct sk_buff *skb, unsigned int doff,
|
synproxy_parse_options(const struct sk_buff *skb, unsigned int doff,
|
||||||
const struct tcphdr *th, struct synproxy_options *opts)
|
const struct tcphdr *th, struct synproxy_options *opts)
|
||||||
{
|
{
|
||||||
@@ -32,7 +32,8 @@ synproxy_parse_options(const struct sk_buff *skb, unsigned int doff,
|
|||||||
u8 buf[40], *ptr;
|
u8 buf[40], *ptr;
|
||||||
|
|
||||||
ptr = skb_header_pointer(skb, doff + sizeof(*th), length, buf);
|
ptr = skb_header_pointer(skb, doff + sizeof(*th), length, buf);
|
||||||
BUG_ON(ptr == NULL);
|
if (ptr == NULL)
|
||||||
|
return false;
|
||||||
|
|
||||||
opts->options = 0;
|
opts->options = 0;
|
||||||
while (length > 0) {
|
while (length > 0) {
|
||||||
@@ -41,16 +42,16 @@ synproxy_parse_options(const struct sk_buff *skb, unsigned int doff,
|
|||||||
|
|
||||||
switch (opcode) {
|
switch (opcode) {
|
||||||
case TCPOPT_EOL:
|
case TCPOPT_EOL:
|
||||||
return;
|
return true;
|
||||||
case TCPOPT_NOP:
|
case TCPOPT_NOP:
|
||||||
length--;
|
length--;
|
||||||
continue;
|
continue;
|
||||||
default:
|
default:
|
||||||
opsize = *ptr++;
|
opsize = *ptr++;
|
||||||
if (opsize < 2)
|
if (opsize < 2)
|
||||||
return;
|
return true;
|
||||||
if (opsize > length)
|
if (opsize > length)
|
||||||
return;
|
return true;
|
||||||
|
|
||||||
switch (opcode) {
|
switch (opcode) {
|
||||||
case TCPOPT_MSS:
|
case TCPOPT_MSS:
|
||||||
@@ -84,6 +85,7 @@ synproxy_parse_options(const struct sk_buff *skb, unsigned int doff,
|
|||||||
length -= opsize;
|
length -= opsize;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
return true;
|
||||||
}
|
}
|
||||||
EXPORT_SYMBOL_GPL(synproxy_parse_options);
|
EXPORT_SYMBOL_GPL(synproxy_parse_options);
|
||||||
|
|
||||||
|
Reference in New Issue
Block a user