Change in osmo-ggsn[master]: replace bogus memcpy() call in ippool_newip()

[Stefan Sperling]

Stefan Sperling has submitted this change and it was merged. ( https://gerrit.osmocom.org/11861 )

Change subject: replace bogus memcpy() call in ippool_newip()
......................................................................

replace bogus memcpy() call in ippool_newip()

When copying an address to a reused static hash table member
with memcpy(), this code mistakenly passed the size of a
pointer as the amount of bytes to be copied, rather than
the actual size of the address.

This means the IP pool could contain bogus IP addresses because
only addr->len (a uint8_t) and 3 further bytes of the address
were actually copied on 32 bit platforms. On 64 bit platforms,
a sufficient amount of bytes were copied for IPv4 to work
correctly, but too few bytes were copied for IPv6.

This problem was found by Coverity.

Replace the bogus memcpy() call with direct assignments to the
appropriate struct in64addr union members, and assert that the
length recorded for the address actually corresponds to the
length used by the address family (IP4, IPv6).

Change-Id: Ic21560f7519e776107485a8779702fb1279d065c
Related: CID#57921
---
M lib/ippool.c
1 file changed, 9 insertions(+), 1 deletion(-)

Approvals:
  Pau Espin Pedrol: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/lib/ippool.c b/lib/ippool.c
index 6ce3cda..36121ee 100644
--- a/lib/ippool.c
+++ b/lib/ippool.c
@@ -512,7 +512,15 @@
 		p2->next = NULL;
 		p2->prev = NULL;
 		p2->inuse = 2;	/* Static address in use */
-		memcpy(&p2->addr, addr, sizeof(addr));
+		/* p2->addr.len and addr->len already match (see above). */
+		if (p2->addr.len == sizeof(struct in_addr))
+			p2->addr.v4 = addr->v4;
+		else if (p2->addr.len == sizeof(struct in6_addr))
+			p2->addr.v6 = addr->v6;
+		else {
+			SYS_ERR(DIP, LOGL_ERROR, 0, "MS requested unsupported PDP context type");
+			return -GTPCAUSE_UNKNOWN_PDP;
+		}
 		*member = p2;
 		(void)ippool_hashadd(this, *member);
 		if (0)

-- 
To view, visit https://gerrit.osmocom.org/11861
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: Ic21560f7519e776107485a8779702fb1279d065c
Gerrit-Change-Number: 11861
Gerrit-PatchSet: 3
Gerrit-Owner: Stefan Sperling <ssperling at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: Stefan Sperling <ssperling at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181122/eb6a2e5e/attachment.htm>