Commit [1] fixed the reassembly size cap to use dp_packet_size() rather
than the IPv4 total-length or IPv6 payload-length of the first fragment.
Those L3 fields describe only that fragment and exclude the Ethernet
header, so they understated the buffer size being reassembled into.
That check still uses uint16_t start/end offsets per fragment. If a
fragment's data ends beyond byte 65535, the end offset wraps and the
stored bounds are wrong. rest_len can become negative while
orig_len + rest_len still fits under the cap, so reassembly proceeds on
a corrupt fragment list.
The wrapped bounds can still satisfy ipf_list_complete(). The negative
rest_len is then passed as size_t to dp_packet_prealloc_tailroom(), and
add_len wraps to a huge size_t in dp_packet_put(), causing out-of-
bounds reads and writes on the reassembly buffer and heap corruption.
Reject such fragments when extracting keys and fail reassembly when
rest_len is not positive.
[1] c3f4d9fe54f4 ("ipf: Fix the over-sized reassembly.")
Assisted-by: composer-2.5-fast, Cursor
Fixes: 4ea96698f667 ("Userspace datapath: Add fragmentation handling.")
Signed-off-by: Eli Britstein <[email protected]>
---
lib/ipf.c | 46 ++++++++++++++++++++++++++++++++-----------
tests/ofproto-dpif.at | 45 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 80 insertions(+), 11 deletions(-)
diff --git a/lib/ipf.c b/lib/ipf.c
index d836b8824..274c02f84 100644
--- a/lib/ipf.c
+++ b/lib/ipf.c
@@ -434,7 +434,7 @@ ipf_reassemble_v4_frags(struct ipf_list *ipf_list)
int rest_len = frag_list[ipf_list->last_inuse_idx].end_data_byte -
frag_list[1].start_data_byte + 1;
- if (orig_len + rest_len > IPV4_PACKET_MAX_SIZE) {
+ if (rest_len <= 0 || orig_len + rest_len > IPV4_PACKET_MAX_SIZE) {
ipf_print_reass_packet(
"Unsupported big reassembled v4 packet; v4 hdr:", l3);
dp_packet_delete(pkt);
@@ -484,7 +484,7 @@ ipf_reassemble_v6_frags(struct ipf_list *ipf_list)
int rest_len = frag_list[ipf_list->last_inuse_idx].end_data_byte -
frag_list[1].start_data_byte + 1;
- if (orig_len + rest_len > IPV6_PACKET_MAX_DATA) {
+ if (rest_len <= 0 || orig_len + rest_len > IPV6_PACKET_MAX_DATA) {
ipf_print_reass_packet(
"Unsupported big reassembled v6 packet; v6 hdr:", l3);
dp_packet_delete(pkt);
@@ -668,9 +668,17 @@ ipf_v4_key_extract(struct dp_packet *pkt, ovs_be16
dl_type, uint16_t zone,
const struct ip_header *l3 = dp_packet_l3(pkt);
uint16_t ip_tot_len = ntohs(l3->ip_tot_len);
size_t ip_hdr_len = IP_IHL(l3->ip_ihl_ver) * 4;
+ uint32_t start = ntohs(l3->ip_frag_off & htons(IP_FRAG_OFF_MASK)) * 8;
- *start_data_byte = ntohs(l3->ip_frag_off & htons(IP_FRAG_OFF_MASK)) * 8;
- *end_data_byte = *start_data_byte + ip_tot_len - ip_hdr_len - 1;
+ /* Fragments with no data or with data ending past 65535 bytes would
+ * wrap the uint16_t fragment bounds, corrupting reassembly. */
+ if (ip_tot_len <= ip_hdr_len
+ || start + (ip_tot_len - ip_hdr_len) - 1 > UINT16_MAX) {
+ return false;
+ }
+
+ *start_data_byte = start;
+ *end_data_byte = start + (ip_tot_len - ip_hdr_len) - 1;
*ff = ipf_is_first_v4_frag(pkt);
*lf = ipf_is_last_v4_frag(pkt);
memset(key, 0, sizeof *key);
@@ -742,7 +750,7 @@ invalid_pkt:
}
-static void
+static bool
ipf_v6_key_extract(struct dp_packet *pkt, ovs_be16 dl_type, uint16_t zone,
struct ipf_list_key *key, uint16_t *start_data_byte,
uint16_t *end_data_byte, bool *ff, bool *lf)
@@ -758,9 +766,18 @@ ipf_v6_key_extract(struct dp_packet *pkt, ovs_be16
dl_type, uint16_t zone,
NULL);
ovs_assert(nw_frag && frag_hdr);
ovs_be16 ip6f_offlg = frag_hdr->ip6f_offlg;
- *start_data_byte = ntohs(ip6f_offlg & IP6F_OFF_MASK) +
+ uint32_t start = ntohs(ip6f_offlg & IP6F_OFF_MASK) +
sizeof (struct ovs_16aligned_ip6_frag);
- *end_data_byte = *start_data_byte + dp_packet_l4_size(pkt) - 1;
+ size_t l4_size = dp_packet_l4_size(pkt);
+
+ /* As in ipf_v4_key_extract(), reject fragments that would wrap the
+ * uint16_t fragment bounds. */
+ if (!l4_size || start + l4_size - 1 > UINT16_MAX) {
+ return false;
+ }
+
+ *start_data_byte = start;
+ *end_data_byte = start + l4_size - 1;
*ff = ipf_is_first_v6_frag(ip6f_offlg);
*lf = ipf_is_last_v6_frag(ip6f_offlg);
memset(key, 0, sizeof *key);
@@ -773,6 +790,7 @@ ipf_v6_key_extract(struct dp_packet *pkt, ovs_be16 dl_type,
uint16_t zone,
key->nw_proto = 0; /* Not used for key for V6. */
key->zone = zone;
key->recirc_id = pkt->md.recirc_id;
+ return true;
}
static bool
@@ -892,11 +910,17 @@ ipf_handle_frag(struct ipf *ipf, struct dp_packet *pkt,
ovs_be16 dl_type,
bool v6 = dl_type == htons(ETH_TYPE_IPV6);
if (v6 && ipf_get_v6_enabled(ipf)) {
- ipf_v6_key_extract(pkt, dl_type, zone, &key, &start_data_byte,
- &end_data_byte, &ff, &lf);
+ if (!ipf_v6_key_extract(pkt, dl_type, zone, &key, &start_data_byte,
+ &end_data_byte, &ff, &lf)) {
+ pkt->md.ct_state = CS_INVALID;
+ return false;
+ }
} else if (!v6 && ipf_get_v4_enabled(ipf)) {
- ipf_v4_key_extract(pkt, dl_type, zone, &key, &start_data_byte,
- &end_data_byte, &ff, &lf);
+ if (!ipf_v4_key_extract(pkt, dl_type, zone, &key, &start_data_byte,
+ &end_data_byte, &ff, &lf)) {
+ pkt->md.ct_state = CS_INVALID;
+ return false;
+ }
} else {
return false;
}
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 644b7b7fb..7e231b967 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -5671,6 +5671,51 @@ CHECK_COVERAGE([dpif_netdev_output_grow_queues], [1])
OVS_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([ofproto-dpif - fragment handling - reject oversized fragment])
+OVS_VSWITCHD_START
+add_of_ports br0 1 90
+
+AT_DATA([flows.txt], [dnl
+table=0 in_port=90,ip actions=ct(commit),output:1
+])
+AT_CHECK([ovs-ofctl -O OpenFlow11 replace-flows br0 flows.txt])
+
+dnl Last fragment at offset 65528 carrying 28 data bytes: its end offset
+dnl (65555) exceeds the maximum packet size and, truncated to 16 bits,
+dnl wraps to 19. It must not be admitted for reassembly.
+dnl IPv4 Packet content:
+dnl Ethernet II, Src: 50:54:00:00:00:09, Dst: 50:54:00:00:00:0a
+dnl Type: IPv4 (0x0800)
+dnl Internet Protocol Version 4, Src: 10.1.1.1, Dst: 10.1.1.2
+dnl 0100 .... = Version: 4
+dnl .... 0101 = Header Length: 20 bytes (5)
+dnl Differentiated Services Field: 0x00 (DSCP: CS0, ECN: Not-ECT)
+dnl Total Length: 48
+dnl Identification: 0x0001 (1)
+dnl 001. .... = Flags: 0x1, More fragments
+dnl 0... .... = Reserved bit: Not set
+dnl .0.. .... = Don't fragment: Not set
+dnl ..1. .... = More fragments: Set
+dnl ...1 1111 1111 1111 = Fragment Offset: 65528
+dnl Time to Live: 64
+dnl Protocol: UDP (17)
+dnl Header Checksum: 0x44b9
+dnl Data (28 bytes)
+eth="50 54 00 00 00 0a 50 54 00 00 00 09 08 00"
+ip="45 00 00 30 00 01 1f ff 40 11 44 b9 0a 01 01 01 0a 01 01 02"
+data="00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12 13 14 15 16 17
18 19 1a 1b"
+packet="${eth} ${ip} ${data}"
+AT_CHECK([ovs-appctl netdev-dummy/receive p90 "$packet"])
+
+AT_CHECK([ovs-appctl dpctl/ipf-get-status -m | grep -E 'num frag:|frags
accepted:'], [], [dnl
+ num frag: 0
+ v4 frags accepted: 0
+ v6 frags accepted: 0
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([ofproto-dpif - handling of malformed TCP packets])
OVS_VSWITCHD_START
add_of_ports br0 1 90
--
2.43.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
