| Summary: | bindresvport_sa() does not validate non-zero struct sockaddr * port is within intended range | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Portable OpenSSH | Reporter: | Glenn <gs-bugzilla.mindrot.org> | ||||
| Component: | Miscellaneous | Assignee: | Assigned to nobody <unassigned-bugs> | ||||
| Status: | CLOSED FIXED | ||||||
| Severity: | normal | CC: | dtucker | ||||
| Priority: | P2 | ||||||
| Version: | 5.8p2 | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Attachments: |
|
||||||
openssh appears unaffected since the only use is in sshconnect.c, which calls rresvport_af() with a parameter that results in bindresvport_sa() being called with port set to 0 in struct sockaddr *. port == 0 is handled properly, as expected, in bindresvport_sa(). We fixed this in a manner of speaking by removing support for running ssh as root and binding to a low-numbered ports in OpenSSH 7.8. https://www.openssh.com/releasenotes.html#7.8 says: """ * ssh(1): remove vestigal support for running ssh(1) as setuid. This used to be required for hostbased authentication and the (long gone) rhosts-style authentication, but has not been necessary for a long time. Attempting to execute ssh as a setuid binary, or with uid != effective uid will now yield a fatal error at runtime. """ For anyone still requiring the low-numbered port functionality for legacy reasons we recommend a small setuid helper ProxyCommand or some source-NAT trickery. Thanks for the report. Close bugs fixed in openssh-8.1 release cycle |
Created attachment 2043 [details] validate that port provided in struct sockaddr * to bindresvport_sa() is within valid range openbsd-compat/bindresvport.c does not validate that the port provided in struct sockaddr * to bindresvport_sa() is within the valid range of intended reserved ports. If the port provided is non-zero, then that is taken as the starting port. For values >= IPPORT_RESERVED, it has the effect of attempting to bind() to that port (higher than the reserved port range) and then, if that port is in use, tries NPORTS-1 from STARTPORT. If all ports [STARTPORT,ENDPORT) are in use, ENDPORT is never tried. For non-zero values < STARTPORT, there are potentially many lower ports that are tried, and even the possibility that no ports within the intended range are tried. If a free port is found, the result is bind()ing to a port < IPPORT_RESERVED, but possibly not within the intended range of ports. Trivial fix attached: change 'if (port == 0)' to 'if (port < STARTPORT || port > ENDPORT)' Cheers, Glenn