Bug 1903 - bindresvport_sa() does not validate non-zero struct sockaddr * port is within intended range
Summary: bindresvport_sa() does not validate non-zero struct sockaddr * port is withi...
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: Miscellaneous (show other bugs)
Version: 5.8p2
Hardware: All All
: P2 normal
Assignee: Assigned to nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-10 13:49 AEST by Glenn
Modified: 2019-10-09 15:11 AEDT (History)
1 user (show)

See Also:


Attachments
validate that port provided in struct sockaddr * to bindresvport_sa() is within valid range (472 bytes, application/octet-stream)
2011-05-10 13:49 AEST, Glenn
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Glenn 2011-05-10 13:49:39 AEST
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
Comment 1 Glenn 2011-05-10 13:53:15 AEST
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().
Comment 2 Darren Tucker 2019-01-23 20:00:00 AEDT
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.
Comment 3 Damien Miller 2019-10-09 15:11:42 AEDT
Close bugs fixed in openssh-8.1 release cycle