Change in osmo-ggsn[master]: ggsn: fix misinterpreted length field in ipcp_contains_option()

[Harald Welte]

Harald Welte has submitted this change and it was merged. ( https://gerrit.osmocom.org/9354 )

Change subject: ggsn: fix misinterpreted length field in ipcp_contains_option()
......................................................................

ggsn: fix misinterpreted length field in ipcp_contains_option()

The abort condition of the while loop in ipcp_contains_option()
is accessing ipcp->len directly. Unfortunately this field is an
uint16_t which as to be interpreted as little endian value. If
it is used without prior conversion the value may appear larger
than actually intended and the loop will then not stop at the
end of end of the buffer.

This can cause unpredictable results when the value given with
the parameter enum ipcp_options opt is not found.

The loop will then eventually cause a segmentation fauld or
is likely to hang as soon as cur_opt->len points to a zero
byte in memory.

- Make sure that ipcp->len interpreted correctly by accessing
  it through ntohs()

Change-Id: Icffde89f9bc5d8fcadf6e2dd6c0b4de03440edd5
Related: OS#3288
---
M ggsn/ggsn.c
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Jenkins Builder: Verified
  Pau Espin Pedrol: Looks good to me, but someone else must approve
  Harald Welte: Looks good to me, approved



diff --git a/ggsn/ggsn.c b/ggsn/ggsn.c
index 72bf61c..3a8c4be 100644
--- a/ggsn/ggsn.c
+++ b/ggsn/ggsn.c
@@ -418,7 +418,7 @@
 	uint8_t *cur = ipcp->options;
 
 	/* iterate over Options and check if protocol contained */
-	while (cur + 2 <= ((uint8_t *)ipcp) + ipcp->len) {
+	while (cur + 2 <= ((uint8_t *)ipcp) + ntohs(ipcp->len)) {
 		struct ipcp_option_hdr *cur_opt = (struct ipcp_option_hdr *) cur;
 		if (cur_opt->type == opt)
 			return cur_opt;

-- 
To view, visit https://gerrit.osmocom.org/9354
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-ggsn
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Icffde89f9bc5d8fcadf6e2dd6c0b4de03440edd5
Gerrit-Change-Number: 9354
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20180531/75103ce1/attachment.htm>