d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Avoid buffer underwrite in icmp_error

Browse Source 
icmp_error allocates either an mbuf (with pkthdr) or a cluster depending
on the size of data to be quoted in the ICMP reply, but the calculation
failed to account for the additional padding that m_align may apply.

Include the ip header in the size passed to m_align.  On 64-bit archs
this will have the net effect of moving everything 4 bytes later in the
mbuf or cluster.  This will result in slightly pessimal alignment for
the ICMP data copy.

Also add an assertion that we do not move m_data before the beginning of
the mbuf or cluster.

Reported by:	A reddit user
Reviewed by:	bz, jtl
MFC after:	3 days
Security:	CVE-2018-17156
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D17909
This commit is contained in:
emaste 2018-11-08 20:17:36 +00:00
parent 490f74685f
commit a9fb9d9a30
1 changed files with 4 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
sys/netinet/ip_icmp.c
View File

@ -320,7 +320,8 @@ stdreply: icmpelen = max(8, min(V_icmp_quotelen, ntohs(oip->ip_len) -
#endif
icmplen = min(icmplen, M_TRAILINGSPACE(m) -
sizeof(struct ip) - ICMP_MINLEN);
m_align(m, ICMP_MINLEN + icmplen);
m_align(m, sizeof(struct ip) + ICMP_MINLEN + icmplen);
m->m_data += sizeof(struct ip);
m->m_len = ICMP_MINLEN + icmplen;
/* XXX MRT make the outgoing packet use the same FIB
@ -362,6 +363,8 @@ stdreply: icmpelen = max(8, min(V_icmp_quotelen, ntohs(oip->ip_len) -
* reply should bypass as well.
*/
m->m_flags |= n->m_flags & M_SKIP_FIREWALL;
KASSERT(M_LEADINGSPACE(m) >= sizeof(struct ip),
("insufficient space for ip header"));
m->m_data -= sizeof(struct ip);
m->m_len += sizeof(struct ip);
m->m_pkthdr.len = m->m_len;