n0k0 has uploaded this change for review.
ipaccess-proxy: reject short IPA header in ipaccess_proxy_read_msg
recv() of the 3-byte IPA header can return fewer than 3 bytes on a
stream socket. ipaccess_proxy_read_msg() only handles ret < 0 and
ret == 0, so a short read falls through: msgb_put() advances msg->tail
by 'ret' while msg->l2h is set to msg->data + sizeof(*hh). The frame
length is then validated against msgb_tailroom() (measured from
msg->tail), so the subsequent body recv() into msg->l2h can write up to
sizeof(*hh) - ret bytes past the msgb.
Reject a short header, the same way handle_udp_read() already does.
Change-Id: I3043cccd110db19984f47916c269a4c59f82b08b
---
M src/ipaccess/ipaccess-proxy.c
1 file changed, 10 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/88/43188/1
diff --git a/src/ipaccess/ipaccess-proxy.c b/src/ipaccess/ipaccess-proxy.c
index 71190f6..b1c500c 100644
--- a/src/ipaccess/ipaccess-proxy.c
+++ b/src/ipaccess/ipaccess-proxy.c
@@ -576,6 +576,16 @@
msgb_free(msg);
*error = ret;
return NULL;
+ } else if (ret < sizeof(*hh)) {
+ /* A short (split) IPA header would leave msg->l2h (msg->data +
+ * sizeof(*hh)) ahead of msg->tail, while the frame length below is
+ * validated against msgb_tailroom() measured from msg->tail; the
+ * body recv() could then write up to sizeof(*hh) - ret bytes past
+ * the buffer. Reject it, like the other IPA read paths do. */
+ LOGP(DLINP, LOGL_ERROR, "short read of IPA header (%d)\n", ret);
+ msgb_free(msg);
+ *error = -EIO;
+ return NULL;
}
msgb_put(msg, ret);
To view, visit change 43188. To unsubscribe, or for help writing mail filters, visit settings.