2017-03-03 14:08 GMT-08:00 Ben Pfaff <[email protected]>:
> Otherwise a malformed packet could cause a read up to about 40 bytes past
> the end of the packet. The packet would still likely be dropped because
> of checksum verification.
>
> Reported-by: Bhargava Shastry <[email protected]>
> Signed-off-by: Ben Pfaff <[email protected]>
Oops, thanks for the fix, Ben
Fixes: a489b16854b5("conntrack: New userspace connection tracker.")
One minor comment below,
Acked-by: Daniele Di Proietto <[email protected]>
> ---
> lib/conntrack.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 9bea3d93e4ad..9c1dd63648b8 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -568,6 +568,10 @@ extract_l3_ipv6(struct conn_key *key, const void *data,
> size_t size,
> const char **new_data)
> {
> const struct ovs_16aligned_ip6_hdr *ip6 = data;
> + if (size < sizeof *ip6) {
> + return false;
> + }
> +
We can read 'ip6->ip6_nxt' even though there's not enough data. It
cannot happen
for regular TCP and UDP packets (those are covered my
miniflow_extract), but only
when parsing the nested l3 header in an ICMP error message.
The code has the same check two lines below, maybe we can reuse that.
Technically
the check is necessary only if new_data != NULL, as explained by the comment
above, but perhaps it's more clear to always perform it.
> uint8_t nw_proto = ip6->ip6_nxt;
> uint8_t nw_frag = 0;
>
> @@ -623,8 +627,11 @@ check_l4_tcp(const struct conn_key *key, const void
> *data, size_t size,
> const void *l3)
> {
> const struct tcp_header *tcp = data;
> - size_t tcp_len = TCP_OFFSET(tcp->tcp_ctl) * 4;
> + if (size < sizeof *tcp) {
> + return false;
> + }
>
> + size_t tcp_len = TCP_OFFSET(tcp->tcp_ctl) * 4;
> if (OVS_UNLIKELY(tcp_len < TCP_HEADER_LEN || tcp_len > size)) {
> return false;
> }
> @@ -637,8 +644,11 @@ check_l4_udp(const struct conn_key *key, const void
> *data, size_t size,
> const void *l3)
> {
> const struct udp_header *udp = data;
> - size_t udp_len = ntohs(udp->udp_len);
> + if (size < sizeof *udp) {
> + return false;
> + }
>
> + size_t udp_len = ntohs(udp->udp_len);
> if (OVS_UNLIKELY(udp_len < UDP_HEADER_LEN || udp_len > size)) {
> return false;
> }
> --
> 2.10.2
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
