The extra validation added to fix some security issues in the following change broke handling of the fragmented UDP packets. I've only tested it with UDP but it might also affect TCP and ICMP as well. ---- Author: emaste Date: Tue May 12 16:33:04 2020 New Revision: 360967 URL: https://svnweb.freebsd.org/changeset/base/360967 Log: libalias: validate packet lengths before accessing headers admbugs: 956 Submitted by: ae Reported by: Lucas Leong (@_wmliang_) of Trend Micro Zero Day Initiative Reported by: Vishnu working with Trend Micro Zero Day Initiative Security: FreeBSD-SA-20:12.libalias Modified: head/sys/netinet/libalias/alias.c ---- As a result, ng_nat (in our case) passes fragmented [UDP] packets unaliased, both first fragment and any subsequent ones. This would also affect other users of sys/libalias, not just ng_nat.
(In reply to Maxim Sobolev from comment #0) Hi, I think only UDP is affected. You can remove if (dlen < ntohs(ud->uh_ulen)) .... checks, this should help. But I think you will need to modify upper level protocols, that are based on UDP. They need to have such checks before accessing to UDP data.
(In reply to Andrey V. Elsukov from comment #1) Andrey, removing that check only won't be sufficient, the situation is more complicated than that. There are 3 distinct cases to consider: 1. Normal packet (no fragmentation). In that case both of checks apply. 2. First fragment. In that case the packet has UDP/TCP header but payload may be truncated. Only first check apply. 3. Any of the subsequent fragments. In that case there is just raw data after IP header, so none of the checks apply here. There are two places in the code where this logic applies, (In/Out), so I am working on unifying those checks into a subroutine. I also noticed that logic permits some random data be taken out of the data area and passed into functions as "source" and "destination" ports for the case (3) above. While the logic perhaps just ignores those garbage values it's still looks like a bit of oversight. Stay tuned, I will post review request on phabricator once I finish and test everything. Thanks! -Max
Patch has been posted for a review here: https://reviews.freebsd.org/D32363
Fix committed into the main branch and will be merged into 13/stable 12/stable and 11/stable soon. commit 461e6f23db3b9794e6af88b381b066a2c0463d1c Author: Maxim Sobolev <sobomax@FreeBSD.org> AuthorDate: 2021-10-07 20:41:40 +0000 Commit: Maxim Sobolev <sobomax@FreeBSD.org> CommitDate: 2021-10-15 23:48:12 +0000 Fix fragmented UDP packets handling since rev.360967. Consider IP_MF flag when checking length of the UDP packet to match the declared value. Sponsored by: Sippy Software, Inc. Differential Revision: https://reviews.freebsd.org/D32363 MFC after: 2 weeks
Merged to stable/13 and stable/12: commit ec746e61957840dd26adec945581e66c1b007e99 Author: Maxim Sobolev <sobomax@FreeBSD.org> AuthorDate: Thu Oct 7 13:41:40 2021 -0700 Commit: Ed Maste <emaste@FreeBSD.org> CommitDate: Sun Jan 9 17:04:56 2022 -0500 Fix fragmented UDP packets handling since rev.360967. Consider IP_MF flag when checking length of the UDP packet to match the declared value. Sponsored by: Sippy Software, Inc. Differential Revision: https://reviews.freebsd.org/D32363 MFC after: 2 weeks (cherry picked from commit 461e6f23db3b9794e6af88b381b066a2c0463d1c) commit 73c5a2566dbb3ae57970b203d4de6fcf6088701c Author: Maxim Sobolev <sobomax@FreeBSD.org> AuthorDate: Thu Oct 7 13:41:40 2021 -0700 Commit: Ed Maste <emaste@FreeBSD.org> CommitDate: Sun Jan 9 17:06:00 2022 -0500 Fix fragmented UDP packets handling since rev.360967. Consider IP_MF flag when checking length of the UDP packet to match the declared value. Sponsored by: Sippy Software, Inc. Differential Revision: https://reviews.freebsd.org/D32363 MFC after: 2 weeks (cherry picked from commit 461e6f23db3b9794e6af88b381b066a2c0463d1c)