lynxis lazus has submitted this change. (
https://gerrit.osmocom.org/c/osmo-ggsn/+/43187?usp=email )
Change subject: gtp: clamp GSN-Address copy to sockaddr_in size
......................................................................
gtp: clamp GSN-Address copy to sockaddr_in size
gtp_data_req() builds an IPv4 destination sockaddr_in and does
memcpy(&addr.sin_addr, pdp->gsnru.v, pdp->gsnru.l); gtp_gpdu_ind()
checks the GPDU source with memcmp(&peer->sin_addr, pdp->gsnru.v,
pdp->gsnru.l). Both use the wire GSN-Address length as the count into
the 4-byte sin_addr, so a GSN-Address longer than 4 bytes (e.g. a
16-byte IPv6 address) writes up to 4 bytes past the sockaddr_in on the
stack, or reads up to 12 bytes past sin_addr. Both sites already carried
a 'TODO range check' comment.
This is the AF_INET user-plane path, where the GSN address is a 4-byte
IPv4 address, so copy and compare exactly sizeof(sin_addr) bytes.
Change-Id: If49f1929645c0ba8d19f3c9aa95f57d0a432ad53
---
M gtp/gtp.c
1 file changed, 8 insertions(+), 2 deletions(-)
Approvals:
pespin: Looks good to me, but someone else must approve
lynxis lazus: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/gtp/gtp.c b/gtp/gtp.c
index fa65575..7a439d9 100644
--- a/gtp/gtp.c
+++ b/gtp/gtp.c
@@ -3280,7 +3280,10 @@
}
/* If the GPDU was not from the peer GSN tell him to delete context */
- if (memcmp(&peer->sin_addr, pdp->gsnru.v, pdp->gsnru.l)) { /* TODO
Range? */
+ /* Compare only the IPv4 address bytes; peer->sin_addr is 4 bytes, so
+ * bound the compare to sizeof(sin_addr) rather than the
(attacker-influenced)
+ * gsnru length to avoid reading past sin_addr. */
+ if (memcmp(&peer->sin_addr, pdp->gsnru.v, sizeof(peer->sin_addr))) {
rate_ctr_inc2(gsn->ctrg, GSN_CTR_ERR_UNKNOWN_PDP);
GTP_LOGPKG(LOGL_ERROR, peer, pack, len, "Unknown GSN peer
%s\n", inet_ntoa(peer->sin_addr));
return gtp_error_ind_resp(gsn, version, peer, fd, pack, len);
@@ -3778,7 +3781,10 @@
#if defined(__FreeBSD__) || defined(__APPLE__)
addr.sin_len = sizeof(addr);
#endif
- memcpy(&addr.sin_addr, pdp->gsnru.v, pdp->gsnru.l); /* TODO range
check */
+ /* gsnru is the IPv4 GSN user-plane address for this AF_INET path; copy
+ * exactly sizeof(sin_addr) bytes so an over-long (e.g. 16-byte IPv6)
+ * address cannot write past addr on the stack. */
+ memcpy(&addr.sin_addr, pdp->gsnru.v, sizeof(addr.sin_addr));
/* prepare msghdr */
memset(&msgh, 0, sizeof(msgh));
--
To view, visit https://gerrit.osmocom.org/c/osmo-ggsn/+/43187?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-ggsn
Gerrit-Branch: master
Gerrit-Change-Id: If49f1929645c0ba8d19f3c9aa95f57d0a432ad53
Gerrit-Change-Number: 43187
Gerrit-PatchSet: 1
Gerrit-Owner: n0k0 <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: lynxis lazus <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
