Skip to content

Commit 56b29f8

Browse files
committed
ip4_frag/ip6_frag: fix potential NULL-pointer access on memory errors
1 parent 92522e4 commit 56b29f8

File tree

3 files changed

+50
-41
lines changed

3 files changed

+50
-41
lines changed

CHANGELOG

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ HISTORY
66

77
* [Enter new changes just after this line - do not remove this line]
88

9+
++ Bugfixes:
10+
11+
2025-06-03: Simon Goldschmidt
12+
* ip4_frag/ip6_frag: fix potential NULL-pointer access on memory errors
13+
914
(STABLE-2.2.1):
1015

1116
++ New features:

src/core/ipv4/ip4_frag.c

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -175,19 +175,21 @@ ip_reass_free_complete_datagram(struct ip_reassdata *ipr, struct ip_reassdata *p
175175

176176
MIB2_STATS_INC(mib2.ipreasmfails);
177177
#if LWIP_ICMP
178-
iprh = (struct ip_reass_helper *)ipr->p->payload;
179-
if (iprh->start == 0) {
180-
/* The first fragment was received, send ICMP time exceeded. */
181-
/* First, de-queue the first pbuf from r->p. */
182-
p = ipr->p;
183-
ipr->p = iprh->next_pbuf;
184-
/* Then, copy the original header into it. */
185-
SMEMCPY(p->payload, &ipr->iphdr, IP_HLEN);
186-
icmp_time_exceeded(p, ICMP_TE_FRAG);
187-
clen = pbuf_clen(p);
188-
LWIP_ASSERT("pbufs_freed + clen <= 0xffff", pbufs_freed + clen <= 0xffff);
189-
pbufs_freed = (u16_t)(pbufs_freed + clen);
190-
pbuf_free(p);
178+
if (ipr->p != NULL) {
179+
iprh = (struct ip_reass_helper *)ipr->p->payload;
180+
if (iprh->start == 0) {
181+
/* The first fragment was received, send ICMP time exceeded. */
182+
/* First, de-queue the first pbuf from r->p. */
183+
p = ipr->p;
184+
ipr->p = iprh->next_pbuf;
185+
/* Then, copy the original header into it. */
186+
SMEMCPY(p->payload, &ipr->iphdr, IP_HLEN);
187+
icmp_time_exceeded(p, ICMP_TE_FRAG);
188+
clen = pbuf_clen(p);
189+
LWIP_ASSERT("pbufs_freed + clen <= 0xffff", pbufs_freed + clen <= 0xffff);
190+
pbufs_freed = (u16_t)(pbufs_freed + clen);
191+
pbuf_free(p);
192+
}
191193
}
192194
#endif /* LWIP_ICMP */
193195

src/core/ipv6/ip6_frag.c

Lines changed: 30 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -154,35 +154,37 @@ ip6_reass_free_complete_datagram(struct ip6_reassdata *ipr)
154154
struct ip6_reass_helper *iprh;
155155

156156
#if LWIP_ICMP6
157-
iprh = (struct ip6_reass_helper *)ipr->p->payload;
158-
if (iprh->start == 0) {
159-
/* The first fragment was received, send ICMP time exceeded. */
160-
/* First, de-queue the first pbuf from r->p. */
161-
p = ipr->p;
162-
ipr->p = iprh->next_pbuf;
163-
/* Restore the part that we've overwritten with our helper structure, or we
164-
* might send garbage (and disclose a pointer) in the ICMPv6 reply. */
165-
MEMCPY(p->payload, ipr->orig_hdr, sizeof(*iprh));
166-
/* Then, move back to the original ipv6 header (we are now pointing to Fragment header).
167-
This cannot fail since we already checked when receiving this fragment. */
168-
if (pbuf_header_force(p, (s16_t)((u8_t*)p->payload - (u8_t*)ipr->iphdr))) {
169-
LWIP_ASSERT("ip6_reass_free: moving p->payload to ip6 header failed", 0);
170-
}
171-
else {
172-
/* Reconstruct the zoned source and destination addresses, so that we do
173-
* not end up sending the ICMP response over the wrong link. */
174-
ip6_addr_t src_addr, dest_addr;
175-
ip6_addr_copy_from_packed(src_addr, IPV6_FRAG_SRC(ipr));
176-
ip6_addr_set_zone(&src_addr, ipr->src_zone);
177-
ip6_addr_copy_from_packed(dest_addr, IPV6_FRAG_DEST(ipr));
178-
ip6_addr_set_zone(&dest_addr, ipr->dest_zone);
179-
/* Send the actual ICMP response. */
180-
icmp6_time_exceeded_with_addrs(p, ICMP6_TE_FRAG, &src_addr, &dest_addr);
157+
if (ipr->p != NULL) {
158+
iprh = (struct ip6_reass_helper *)ipr->p->payload;
159+
if (iprh->start == 0) {
160+
/* The first fragment was received, send ICMP time exceeded. */
161+
/* First, de-queue the first pbuf from r->p. */
162+
p = ipr->p;
163+
ipr->p = iprh->next_pbuf;
164+
/* Restore the part that we've overwritten with our helper structure, or we
165+
* might send garbage (and disclose a pointer) in the ICMPv6 reply. */
166+
MEMCPY(p->payload, ipr->orig_hdr, sizeof(*iprh));
167+
/* Then, move back to the original ipv6 header (we are now pointing to Fragment header).
168+
This cannot fail since we already checked when receiving this fragment. */
169+
if (pbuf_header_force(p, (s16_t)((u8_t*)p->payload - (u8_t*)ipr->iphdr))) {
170+
LWIP_ASSERT("ip6_reass_free: moving p->payload to ip6 header failed", 0);
171+
}
172+
else {
173+
/* Reconstruct the zoned source and destination addresses, so that we do
174+
* not end up sending the ICMP response over the wrong link. */
175+
ip6_addr_t src_addr, dest_addr;
176+
ip6_addr_copy_from_packed(src_addr, IPV6_FRAG_SRC(ipr));
177+
ip6_addr_set_zone(&src_addr, ipr->src_zone);
178+
ip6_addr_copy_from_packed(dest_addr, IPV6_FRAG_DEST(ipr));
179+
ip6_addr_set_zone(&dest_addr, ipr->dest_zone);
180+
/* Send the actual ICMP response. */
181+
icmp6_time_exceeded_with_addrs(p, ICMP6_TE_FRAG, &src_addr, &dest_addr);
182+
}
183+
clen = pbuf_clen(p);
184+
LWIP_ASSERT("pbufs_freed + clen <= 0xffff", pbufs_freed + clen <= 0xffff);
185+
pbufs_freed = (u16_t)(pbufs_freed + clen);
186+
pbuf_free(p);
181187
}
182-
clen = pbuf_clen(p);
183-
LWIP_ASSERT("pbufs_freed + clen <= 0xffff", pbufs_freed + clen <= 0xffff);
184-
pbufs_freed = (u16_t)(pbufs_freed + clen);
185-
pbuf_free(p);
186188
}
187189
#endif /* LWIP_ICMP6 */
188190

0 commit comments

Comments
 (0)