openssh unix dev - Dec 2008 - Using realloc to remove MAX_LISTEN_SOCKS limit on sshd.c

OpenSSH developers,

I have removed the fixed, arbitrary limit on the number of ListenAddress 
allowed by using realloc to dynamically expand listen_socks as needed.  
This completely removes MAX_LISTEN_SOCKS from the source.  I made this 
change on the version of OpenSSH shipped with CentOS 5.2, version 
4.3p2.  Please see the attached .c file and .diff file.  Please add 
these changes to OpenSSH to save people from having to predetermine 
their workload before compilation.  It can also save some people some 
grief - I've been unable to login to a server because of this one.

sshd.c.orig is the original file bundled in the CentOS source RPM

sshd.c is the new version

sshd.c.diff is the difference between then: diff sshd.c.orig sshd.c


Thank you,

-- 
Dan Armstrong
AO Industries, Inc.
dan at aoindustries.com
Work: (251) 607-9556
Cell: (205) 454-2556

-------------- next part --------------
A non-text attachment was scrubbed...
Name: sshd.c
Type: text/x-csrc
Size: 56316 bytes
Desc: not available
Url :
http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20081219/51d6495b/attachment-0002.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sshd.c.diff
Type: text/x-patch
Size: 925 bytes
Desc: not available
Url :
http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20081219/51d6495b/attachment-0003.bin
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: sshd.c.orig
Url:
http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20081219/51d6495b/attachment-0001.ksh

Dan Armstrong wrote:
> OpenSSH developers,
>
> I have removed the fixed, arbitrary limit on the number of 
> ListenAddress allowed by using realloc to dynamically expand 
> listen_socks as needed.  This completely removes MAX_LISTEN_SOCKS from 
> the source.  I made this change on the version of OpenSSH shipped with 
> CentOS 5.2, version 4.3p2.  Please see the attached .c file and .diff 
> file.  Please add these changes to OpenSSH to save people from having 
> to predetermine their workload before compilation.  It can also save 
> some people some grief - I've been unable to login to a server because 
> of this one.
Sorry if I may ask the risks of this option - will it not lead to any 
potential scenario of Denial of Service, if some how the number of 
ListenAddress can be arbitrarily increase without limit, and thus 
leading to realloc() allocating large amount of memory?

Thanks.

Hi,

Dan Armstrong wrote:
> I made this change on the version of OpenSSH shipped with CentOS
> 5.2, version 4.3p2.
In the future please make changes against the latest version of the
source code which is available via anonymous CVS. Your change is not
too big, so hopefully it doesn't require very much work to forward
port onto the the latest version.

> Please see the attached .c file and .diff file.
When sending patches in the future please send only the diff output,
and most groups prefer the unified diff format (diff -U) because it
is a lot easier to read.

> 148,149c148,149
> < #define	MAX_LISTEN_SOCKS	16
> < int listen_socks[MAX_LISTEN_SOCKS];
> ---
> > int *listen_socks = NULL;
> > int listen_socks_len = 0;
> 1281,1283d1280
> < 			if (num_listen_socks >= MAX_LISTEN_SOCKS)
> < 				fatal("Too many listen sockets. "
> < 				    "Enlarge MAX_LISTEN_SOCKS");
> 1321a1319,1334
> > 			/* Create/expand listen_socks as needed */
> > 			if(num_listen_socks >= listen_socks_len) {
> > 				int *old_listen_socks = listen_socks;
> > 				/* Start at 16 and then double as needed */
> > 				int new_listen_socks_len = listen_socks_len == 0 ? 16 :
(listen_socks_len << 1);
> > 				listen_socks = realloc(listen_socks, new_listen_socks_len *
sizeof(int));
> > 				if(listen_socks == NULL) {
> > 					free(old_listen_socks);
> > 					old_listen_socks = NULL;
> > 					listen_socks_len = 0;
> > 					fatal("realloc listen_socks: %s", strerror(errno));
> > 				} else {
> > 					listen_socks_len = new_listen_socks_len;
> > 				}
> > 			}
I'm not sure.. I would probably just increase MAX_LISTEN_SOCKS.

Alternatively perhaps the server you couldn't log in to could set
something up with a wildcard listen and a few firewall rules?


Peter Teoh wrote:
> Sorry if I may ask the risks of this option - will it not lead to
> any potential scenario of Denial of Service, if some how the number
> of ListenAddress can be arbitrarily increase without limit, and
> thus leading to realloc() allocating large amount of memory?
It's not a problem. This code runs once at server startup. Whoever is
running sshd could use it to allocate large amounts of memory, but
they could just as easily build another program which does the same
thing. The protection against this situation is to configure limits,
maybe using ulimit.


//Peter