n0k0 has uploaded this change for review.
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(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-ggsn refs/changes/87/43187/1
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 change 43187. To unsubscribe, or for help writing mail filters, visit settings.