From db91cafe5b72f9f591dd8c168427005503186c01 Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Sun, 18 May 2008 21:16:05 +0200 Subject: improve network message sanity checkings --- ChangeLog | 1 + include/network.h | 3 +-- include/sync.h | 1 + src/network.c | 24 ------------------------ src/parse.c | 30 +++++++++++++++++++++++++++--- src/sync-mode.c | 41 ++++++++++++++++++++++++++++++----------- 6 files changed, 60 insertions(+), 40 deletions(-) diff --git a/ChangeLog b/ChangeLog index d67ad30..4458830 100644 --- a/ChangeLog +++ b/ChangeLog @@ -29,6 +29,7 @@ o minor fix of the manpage (Max Wilhelm) o remove (misleading) counters and use information from the statistics mode o use generic nfct_copy() from libnetfilter_conntrack to update objects o use generic nfct_cmp() to compare objects +o improve network message sanity checkings version 0.9.6 (2008/03/08) ------------------------------ diff --git a/include/network.h b/include/network.h index 0fa7b71..baa1eba 100644 --- a/include/network.h +++ b/include/network.h @@ -54,7 +54,6 @@ struct mcast_sock; void build_netmsg(struct nf_conntrack *ct, int query, struct nethdr *net); size_t prepare_send_netmsg(struct mcast_sock *m, void *data); int mcast_send_netmsg(struct mcast_sock *m, void *data); -int handle_netmsg(struct nethdr *net); enum { SEQ_UNKNOWN, @@ -175,6 +174,6 @@ struct netattr { void build_netpld(struct nf_conntrack *ct, struct netpld *pld, int query); -void parse_netpld(struct nf_conntrack *ct, struct netpld *pld, int *query); +int parse_netpld(struct nf_conntrack *ct, struct nethdr *net, int *query, size_t remain); #endif diff --git a/include/sync.h b/include/sync.h index 39e0f46..fc06c93 100644 --- a/include/sync.h +++ b/include/sync.h @@ -20,5 +20,6 @@ struct sync_mode { extern struct sync_mode sync_alarm; extern struct sync_mode sync_ftfw; +extern struct sync_mode sync_notrack; #endif diff --git a/src/network.c b/src/network.c index d7ab415..fb6ea90 100644 --- a/src/network.c +++ b/src/network.c @@ -171,30 +171,6 @@ void build_netmsg(struct nf_conntrack *ct, int query, struct nethdr *net) build_netpld(ct, pld, query); } -int handle_netmsg(struct nethdr *net) -{ - struct netpld *pld = NETHDR_DATA(net); - - /* message too small: no room for the header */ - if (ntohs(net->len) < NETHDR_ACK_SIZ) - return -1; - - HDR_NETWORK2HOST(net); - - if (IS_CTL(net)) - return 0; - - /* information received is too small */ - if (net->len < sizeof(struct netpld)) - return -1; - - /* size mismatch! */ - if (net->len < ntohs(pld->len) + NETHDR_SIZ) - return -1; - - return 0; -} - static int local_seq_set = 0; /* this function only tracks, it does not update the last sequence received */ diff --git a/src/parse.c b/src/parse.c index 8ef2e8d..645420d 100644 --- a/src/parse.c +++ b/src/parse.c @@ -20,6 +20,10 @@ #include +#ifndef ssizeof +#define ssizeof(x) (int)sizeof(x) +#endif + static void parse_u8(struct nf_conntrack *ct, int attr, void *data) { uint8_t *value = (uint8_t *) data; @@ -77,20 +81,40 @@ static parse h[ATTR_MAX] = { [ATTR_REPL_NAT_SEQ_OFFSET_AFTER] = parse_u32, }; -void parse_netpld(struct nf_conntrack *ct, struct netpld *pld, int *query) +int +parse_netpld(struct nf_conntrack *ct, + struct nethdr *net, + int *query, + size_t remain) { int len; struct netattr *attr; + struct netpld *pld; + + if (remain < NETHDR_SIZ + sizeof(struct netpld)) + return -1; + + pld = NETHDR_DATA(net); + + if (remain < NETHDR_SIZ + sizeof(struct netpld) + ntohs(pld->len)) + return -1; + + if (net->len < NETHDR_SIZ + sizeof(struct netpld) + ntohs(pld->len)) + return -1; PLD_NETWORK2HOST(pld); len = pld->len; attr = PLD_DATA(pld); - while (len > 0) { + while (len > ssizeof(struct netattr)) { ATTR_NETWORK2HOST(attr); - h[attr->nta_attr](ct, attr->nta_attr, NTA_DATA(attr)); + if (attr->nta_len > len) + return -1; + if (h[attr->nta_attr]) + h[attr->nta_attr](ct, attr->nta_attr, NTA_DATA(attr)); attr = NTA_NEXT(attr, len); } *query = pld->query; + return 0; } diff --git a/src/sync-mode.c b/src/sync-mode.c index a952a5b..7d73e2f 100644 --- a/src/sync-mode.c +++ b/src/sync-mode.c @@ -34,10 +34,9 @@ #include #include -static void do_mcast_handler_step(struct nethdr *net) +static void do_mcast_handler_step(struct nethdr *net, size_t remain) { int query; - struct netpld *pld = NETHDR_DATA(net); char __ct[nfct_maxsize()]; struct nf_conntrack *ct = (struct nf_conntrack *)(void*) __ct; struct us_conntrack *u; @@ -57,8 +56,11 @@ static void do_mcast_handler_step(struct nethdr *net) memset(ct, 0, sizeof(__ct)); - /* XXX: check for malformed */ - parse_netpld(ct, pld, &query); + if (parse_netpld(ct, net, &query, remain) == -1) { + STATE(malformed)++; + dlog(LOG_ERR, "parsing failed: malformed message"); + return; + } switch(query) { case NFCT_Q_CREATE: @@ -91,6 +93,7 @@ retry: debug_ct(ct, "can't destroy"); break; default: + STATE(malformed)++; dlog(LOG_ERR, "mcast unknown query %d\n", query); break; } @@ -113,24 +116,40 @@ static void mcast_handler(void) if (remain < NETHDR_SIZ) { STATE(malformed)++; + dlog(LOG_WARNING, "no room for header"); break; } if (ntohs(net->len) > remain) { - dlog(LOG_ERR, "fragmented messages"); + STATE(malformed)++; + dlog(LOG_WARNING, "fragmented message"); break; } + if (IS_CTL(net)) { + if (remain < NETHDR_ACK_SIZ) { + STATE(malformed)++; + dlog(LOG_WARNING, "no room for ctl message"); + } + + if (ntohs(net->len) < NETHDR_ACK_SIZ) { + STATE(malformed)++; + dlog(LOG_WARNING, "ctl header too small"); + } + } else { + if (ntohs(net->len) < NETHDR_SIZ) { + STATE(malformed)++; + dlog(LOG_WARNING, "header too small"); + } + } + debug("recv sq: %u fl:%u len:%u (rem:%d)\n", ntohl(net->seq), ntohs(net->flags), ntohs(net->len), remain); - /* sanity check and convert nethdr to host byte order */ - if (handle_netmsg(net) == -1) { - STATE(malformed)++; - return; - } - do_mcast_handler_step(net); + HDR_NETWORK2HOST(net); + + do_mcast_handler_step(net, remain); ptr += net->len; remain -= net->len; } -- cgit v1.2.3