defrag: make frag_end and tracker id conversions explicit

-fsanitize=implicit-conversion flags the frag_offset + data_len
narrowing into the uint16_t frag_end. For both IPv4 and IPv6, compute
frag_end after the too-large check so the sum always fits, and make the
narrowing an explicit cast.

For IPv6 this moves the existing IPV6_MAXPACKET check above the
frag_end assignment. It used to sit at the end of the branch, after
frag_end was already computed. Nothing reads frag_end in between, so
behaviour is the same. Oversize fragments are still dropped with
IPV6_FRAG_PKT_TOO_LARGE, and the wrap can no longer happen, which
matches the IPv4 path.

Also drop the vestigial int32 casts when assigning the uint32_t
tracker id.

Ticket: 8232
pull/16253/head
Samaresh Kumar Singh 2 months ago committed by Victor Julien
parent 38c9b1e0e0
commit e6a45d6e4e

@ -130,11 +130,11 @@ static void DefragTrackerInit(DefragTracker *dt, Packet *p)
if (PacketIsIPv4(p)) { if (PacketIsIPv4(p)) {
const IPV4Hdr *ip4h = PacketGetIPv4(p); const IPV4Hdr *ip4h = PacketGetIPv4(p);
dt->id = (int32_t)IPV4_GET_RAW_IPID(ip4h); dt->id = IPV4_GET_RAW_IPID(ip4h);
dt->af = AF_INET; dt->af = AF_INET;
} else { } else {
DEBUG_VALIDATE_BUG_ON(!PacketIsIPv6(p)); DEBUG_VALIDATE_BUG_ON(!PacketIsIPv6(p));
dt->id = (int32_t)IPV6_EXTHDR_GET_FH_ID(p); dt->id = IPV6_EXTHDR_GET_FH_ID(p);
dt->af = AF_INET6; dt->af = AF_INET6;
} }
dt->proto = PacketGetIPProto(p); dt->proto = PacketGetIPProto(p);

@ -591,7 +591,6 @@ DefragInsertFrag(ThreadVars *tv, DecodeThreadVars *dtv, DefragTracker *tracker,
hlen = IPV4_GET_RAW_HLEN(ip4h); hlen = IPV4_GET_RAW_HLEN(ip4h);
data_offset = (uint16_t)((uint8_t *)ip4h + hlen - GET_PKT_DATA(p)); data_offset = (uint16_t)((uint8_t *)ip4h + hlen - GET_PKT_DATA(p));
data_len = IPV4_GET_RAW_IPLEN(ip4h) - hlen; data_len = IPV4_GET_RAW_IPLEN(ip4h) - hlen;
frag_end = frag_offset + data_len;
ip_hdr_offset = (uint16_t)((uint8_t *)ip4h - GET_PKT_DATA(p)); ip_hdr_offset = (uint16_t)((uint8_t *)ip4h - GET_PKT_DATA(p));
/* Ignore fragment if the end of packet extends past the /* Ignore fragment if the end of packet extends past the
@ -600,6 +599,8 @@ DefragInsertFrag(ThreadVars *tv, DecodeThreadVars *dtv, DefragTracker *tracker,
ENGINE_SET_EVENT(p, IPV4_FRAG_PKT_TOO_LARGE); ENGINE_SET_EVENT(p, IPV4_FRAG_PKT_TOO_LARGE);
return NULL; return NULL;
} }
/* the check above ensures the sum fits in frag_end */
frag_end = (uint16_t)(frag_offset + data_len);
} }
else if (tracker->af == AF_INET6) { else if (tracker->af == AF_INET6) {
const IPV6Hdr *ip6h = PacketGetIPv6(p); const IPV6Hdr *ip6h = PacketGetIPv6(p);
@ -607,7 +608,15 @@ DefragInsertFrag(ThreadVars *tv, DecodeThreadVars *dtv, DefragTracker *tracker,
frag_offset = IPV6_EXTHDR_GET_FH_OFFSET(p); frag_offset = IPV6_EXTHDR_GET_FH_OFFSET(p);
data_offset = p->l3.vars.ip6.eh.fh_data_offset; data_offset = p->l3.vars.ip6.eh.fh_data_offset;
data_len = p->l3.vars.ip6.eh.fh_data_len; data_len = p->l3.vars.ip6.eh.fh_data_len;
frag_end = frag_offset + data_len;
/* Ignore fragment if the end of packet extends past the
* maximum size of a packet. */
if (frag_offset + data_len > IPV6_MAXPACKET) {
ENGINE_SET_EVENT(p, IPV6_FRAG_PKT_TOO_LARGE);
return NULL;
}
/* the check above ensures the sum fits in frag_end */
frag_end = (uint16_t)(frag_offset + data_len);
ip_hdr_offset = (uint16_t)((uint8_t *)ip6h - GET_PKT_DATA(p)); ip_hdr_offset = (uint16_t)((uint8_t *)ip6h - GET_PKT_DATA(p));
frag_hdr_offset = p->l3.vars.ip6.eh.fh_header_offset; frag_hdr_offset = p->l3.vars.ip6.eh.fh_header_offset;
@ -629,13 +638,6 @@ DefragInsertFrag(ThreadVars *tv, DecodeThreadVars *dtv, DefragTracker *tracker,
ip6_nh_set_value = IPV6_EXTHDR_GET_FH_NH(p); ip6_nh_set_value = IPV6_EXTHDR_GET_FH_NH(p);
SCLogDebug("offset %d, value %u", ip6_nh_set_offset, ip6_nh_set_value); SCLogDebug("offset %d, value %u", ip6_nh_set_offset, ip6_nh_set_value);
} }
/* Ignore fragment if the end of packet extends past the
* maximum size of a packet. */
if (frag_offset + data_len > IPV6_MAXPACKET) {
ENGINE_SET_EVENT(p, IPV6_FRAG_PKT_TOO_LARGE);
return NULL;
}
} }
else { else {
DEBUG_VALIDATE_BUG_ON(1); DEBUG_VALIDATE_BUG_ON(1);

Loading…
Cancel
Save