diff options
| author | Harald Welte <laforge@gnumonks.org> | 2019-04-10 15:48:26 +0200 |
|---|---|---|
| committer | Harald Welte <laforge@gnumonks.org> | 2019-04-11 19:27:17 +0200 |
| commit | f653c5bc334fd7841119864ffb77271d4fe62bad (patch) | |
| tree | e8bf101ddaebf01cbebfc3a156f0cab04bdbe315 | |
| parent | 549417e67559410d235d75c403b7652859063c38 (diff) | |
ggsn: Fix build_ipcp_pco() in presence of invalid IPCP content
When build_ipcp_pco() iterated over the PCO list, it didn't use
the "outer" pco length as an increment, but used the "inner" IPCP
length.
If an IPCP message with an invalid "inner" length was being processed
(see pcap file attached to OS#3914), the PCO iteration beyond that
broken IPCP would fail, possibly rendering false hits.
Let's make pco_contains_proto() return a pointer to the the pco_element,
so that the caller can use the outer length as an increment.
Change-Id: I8e9cffde092c8c5824abfaeecb742afcf949802c
Related: OS#3914
| -rw-r--r-- | ggsn/ggsn.c | 13 |
1 files changed, 7 insertions, 6 deletions
diff --git a/ggsn/ggsn.c b/ggsn/ggsn.c index 59523ac..8cd4b05 100644 --- a/ggsn/ggsn.c +++ b/ggsn/ggsn.c @@ -471,8 +471,8 @@ struct pco_element { } __attribute__((packed)); /* determine if PCO contains given protocol */ -static const uint8_t *pco_contains_proto(const struct ul255_t *pco, size_t offset, - uint16_t prot, size_t prot_minlen) +static const struct pco_element *pco_contains_proto(const struct ul255_t *pco, size_t offset, + uint16_t prot, size_t prot_minlen) { const uint8_t *cur = pco->v + 1 /*length*/ + offset; @@ -480,7 +480,7 @@ static const uint8_t *pco_contains_proto(const struct ul255_t *pco, size_t offse while (cur + sizeof(struct pco_element) <= pco->v + pco->l) { const struct pco_element *elem = (const struct pco_element *)cur; if (ntohs(elem->protocol_id) == prot && elem->length >= prot_minlen) - return cur; + return elem; cur += elem->length + sizeof(struct pco_element); } return NULL; @@ -515,7 +515,8 @@ static void build_ipcp_pco(const struct apn_ctx *apn, struct pdp_t *pdp, struct { const struct in46_addr *dns1 = &apn->v4.cfg.dns[0]; const struct in46_addr *dns2 = &apn->v4.cfg.dns[1]; - const uint8_t *ipcp, *pco_ipcp; + const struct pco_element *pco_ipcp; + const uint8_t *ipcp; uint16_t ipcp_len; uint8_t *len1, *len2; unsigned int len_appended; @@ -527,7 +528,7 @@ static void build_ipcp_pco(const struct apn_ctx *apn, struct pdp_t *pdp, struct while (pco_ipcp) { uint8_t *start = msg->tail; - ipcp = (pco_ipcp + 3); /* 2=type + 1=len */ + ipcp = pco_ipcp->data; consumed = (ipcp - &pdp->pco_req.v[0]); remain = sizeof(pdp->pco_req.v) - consumed; ipcp_len = osmo_load16be(ipcp + 2); /* 1=code + 1=id */ @@ -560,7 +561,7 @@ static void build_ipcp_pco(const struct apn_ctx *apn, struct pdp_t *pdp, struct *len1 = len_appended - 3; *len2 = len_appended - 3; - offset += 3 + ipcp_len; + offset += sizeof(pco_ipcp) + pco_ipcp->length; pco_ipcp = pco_contains_proto(&pdp->pco_req, offset, PCO_P_IPCP, sizeof(struct ipcp_hdr)); } |