n0k0 has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/43188?usp=email )

Change subject: ipaccess-proxy: reject short IPA header in ipaccess_proxy_read_msg ......................................................................

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);