From a6bc04d59130c49a1dbfadffa4285b11e2ff4939 Mon Sep 17 00:00:00 2001 From: Ondrej Zajicek Date: Thu, 29 Oct 2009 23:57:42 +0100 Subject: Implements better checks on incoming packets and LSAs in OSPF. --- proto/ospf/lsupd.c | 69 +++++++++++++++++++++++++++++++----------------------- 1 file changed, 40 insertions(+), 29 deletions(-) (limited to 'proto/ospf/lsupd.c') diff --git a/proto/ospf/lsupd.c b/proto/ospf/lsupd.c index 5278f1b..7ae2f2a 100644 --- a/proto/ospf/lsupd.c +++ b/proto/ospf/lsupd.c @@ -16,6 +16,7 @@ struct ospf_lsupd_packet }; +/* Beware of unaligned access */ void ospf_dump_lsahdr(struct proto *p, struct ospf_lsa_header *lsa_n) { struct ospf_lsa_header lsa; @@ -38,15 +39,16 @@ static void ospf_dump_lsupd(struct proto *p, struct ospf_lsupd_packet *pkt) ASSERT(op->type == LSUPD_P); ospf_dump_common(p, op); + /* We know that ntohs(op->length) >= sizeof(struct ospf_lsa_header) */ u8 *pbuf= (u8 *) pkt; - int offset = sizeof(struct ospf_lsupd_packet); - int bound = ntohs(op->length) - sizeof(struct ospf_lsa_header); - int i, j; + unsigned int offset = sizeof(struct ospf_lsupd_packet); + unsigned int bound = ntohs(op->length) - sizeof(struct ospf_lsa_header); + unsigned int i, j; j = ntohl(pkt->lsano); for (i = 0; i < j; i++) { - if (offset > bound) + if ((offset > bound) || ((offset % 4) != 0)) { log(L_TRACE "%s: LSA invalid", p->name); return; @@ -405,50 +407,51 @@ void ospf_lsupd_receive(struct ospf_packet *ps_i, struct ospf_iface *ifa, struct ospf_neighbor *n) { - struct ospf_lsupd_packet *ps = (void *) ps_i; + struct ospf_neighbor *ntmp; - struct ospf_lsa_header *lsa; struct proto_ospf *po = ifa->oa->po; struct proto *p = &po->proto; - unsigned int i, sendreq = 1, size = ntohs(ps->ospf_packet.length); + unsigned int i, max, sendreq = 1; - OSPF_PACKET(ospf_dump_lsupd, ps, "LSUPD packet received from %I via %s", n->ip, ifa->iface->name); - - if (n->state < NEIGHBOR_EXCHANGE) + unsigned int size = ntohs(ps_i->length); + if (size < (sizeof(struct ospf_lsupd_packet) + sizeof(struct ospf_lsa_header))) { - OSPF_TRACE(D_PACKETS, "Received lsupd in lesser state than EXCHANGE from (%I)", n->ip); + log(L_ERR "Bad OSPF LSUPD packet from %I - too short (%u B)", n->ip, size); return; } - if (size <= - (sizeof(struct ospf_lsupd_packet) + sizeof(struct ospf_lsa_header))) + struct ospf_lsupd_packet *ps = (void *) ps_i; + OSPF_PACKET(ospf_dump_lsupd, ps, "LSUPD packet received from %I via %s", n->ip, ifa->iface->name); + + if (n->state < NEIGHBOR_EXCHANGE) { - log(L_WARN "Received lsupd from %I is too short!", n->ip); + OSPF_TRACE(D_PACKETS, "Received lsupd in lesser state than EXCHANGE from (%I)", n->ip); return; } ospf_neigh_sm(n, INM_HELLOREC); /* Questionable */ - lsa = (struct ospf_lsa_header *) (ps + 1); + unsigned int offset = sizeof(struct ospf_lsupd_packet); + unsigned int bound = size - sizeof(struct ospf_lsa_header); - for (i = 0; i < ntohl(ps->lsano); i++, - lsa = (struct ospf_lsa_header *) (((u8 *) lsa) + ntohs(lsa->length))) + max = ntohl(ps->lsano); + for (i = 0; i < max; i++) { struct ospf_lsa_header lsatmp; struct top_hash_entry *lsadb; - unsigned diff = ((u8 *) lsa) - ((u8 *) ps), lenn = ntohs(lsa->length); - u16 chsum; - if (((diff + sizeof(struct ospf_lsa_header)) >= size) - || ((lenn + diff) > size)) + if (offset > bound) { log(L_WARN "Received lsupd from %I is too short!", n->ip); ospf_neigh_sm(n, INM_BADLSREQ); - break; + return; } - if ((lenn <= sizeof(struct ospf_lsa_header)) - || (lenn != (4 * (lenn / 4)))) + struct ospf_lsa_header *lsa = (void *) (((u8 *) ps) + offset); + unsigned int lsalen = ntohs(lsa->length); + + if (((offset + lsalen) > size) || ((lsalen % 4) != 0) || + (lsalen <= sizeof(struct ospf_lsa_header))) { log(L_WARN "Received LSA from %I with bad length", n->ip); ospf_neigh_sm(n, INM_BADLSREQ); @@ -456,14 +459,13 @@ ospf_lsupd_receive(struct ospf_packet *ps_i, struct ospf_iface *ifa, } /* pg 143 (1) */ - chsum = lsa->checksum; + u16 chsum = lsa->checksum; if (chsum != lsasum_check(lsa, NULL)) { log(L_WARN "Received bad lsa checksum from %I: %x %x", n->ip, chsum, lsa->checksum); continue; } - #ifdef OSPFv2 /* pg 143 (2) */ if ((lsa->type < LSA_T_RT) || (lsa->type > LSA_T_EXT)) @@ -520,7 +522,6 @@ ospf_lsupd_receive(struct ospf_packet *ps_i, struct ospf_iface *ifa, if ((lsadb == NULL) || (lsa_comp(&lsatmp, &lsadb->lsa) == CMP_NEWER)) { struct ospf_iface *ift = NULL; - void *body; int self = (lsatmp.rt == p->cf->global->router_id); DBG("PG143(5): Received LSA is newer\n"); @@ -618,10 +619,20 @@ ospf_lsupd_receive(struct ospf_packet *ps_i, struct ospf_iface *ifa, } /* FIXME lsack? */ /* pg 144 (5d) */ - body = - mb_alloc(p->pool, lsatmp.length - sizeof(struct ospf_lsa_header)); + void *body = mb_alloc(p->pool, lsatmp.length - sizeof(struct ospf_lsa_header)); ntohlsab(lsa + 1, body, lsatmp.type, lsatmp.length - sizeof(struct ospf_lsa_header)); + + /* We will do validation check after flooding and + acknowledging given LSA to minimize problems + when communicating with non-validating peer */ + if (lsa_validate(&lsatmp, body) == 0) + { + log(L_WARN "Received invalid LSA from %I", n->ip); + mb_free(body); + continue; + } + lsadb = lsa_install_new(po, &lsatmp, domain, body); DBG("New LSA installed in DB\n"); -- cgit v1.2.3