Bug 258970
| Summary: | Excessive packet validation added into rev.360967 in sys/libalias breaks handling of fragmented [UDP] packets | ||
|---|---|---|---|
| Product: | Base System | Reporter: | Maxim Sobolev <sobomax> |
| Component: | kern | Assignee: | Maxim Sobolev <sobomax> |
| Status: | Closed FIXED | ||
| Severity: | Affects Many People | CC: | ae, emaste, markj, sobomax |
| Priority: | --- | ||
| Version: | CURRENT | ||
| Hardware: | Any | ||
| OS: | Any | ||
|
Description
Maxim Sobolev
2021-10-06 20:08:50 UTC
(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) |