openssh-2.3.0p1 bug: vsprintf("%h") is broken

[Dan Astoorian]

I discovered this in openssh-2.3.0p1; it may affect earlier versions as
well.

Platforms: Solaris 2.5.1 and 8, probably others.

Observed behaviour:
    With -v, when attempting to connect to a host which is not
    listening on the requested port, I noticed that the port number is
    reported as zero in the message:

	Secure connection to hostname on port 0 refused.

Apparent cause:
    At line 671 of ssh.c, the message is logged using a format string of
    "%hu".

    It appears that the vsnprintf() implementation in /bsd-snprintf.c
    handles %hu incorrectly.  It does the following at line 262:
	if (cflags == DP_C_SHORT)
	    value = va_arg (args, short int);

    Similarly for %hd, %hi, %ho and %hx.

    The man page for va_arg under Solaris 8 indicates the following:

    |    It is non-portable to specify a second argument of  char  or
    |    short to  va_arg, because arguments seen by the called func-
    |    tion are not char or short.  C  converts   char  and   short
    |    arguments to  int before passing them to a function.

To reproduce:
    Compile the following against bsd-snprintf.o:

	#include <varargs.h>
	#include <stdio.h>

	char msgbuf[256];

	void foo(char *fmt, ...) {
	    va_list args;
	    va_start(args, fmt);
	    vsnprintf(msgbuf, sizeof(msgbuf), fmt, args);
	    printf("%s", msgbuf);
	}

	main() {
	    int i = 10;
	    short j = 10;
	    foo("x %d x\n", i);
	    foo("x %hu x\n", i);
	    foo("x %d x\n", j);
	    foo("x %hu x\n", j);
	}

    The resulting program prints "x 0 x" instead of "x 10 x" on the
    second and fourth lines.

Remedy:
    Presumably, change "short int" to "int", and "unsigned short int" at
    the relevant lines (263, 275, 287, 301 of bsd-snprintf.c).  If there
    are implementations which break the specification and fail to
    promote arguments of type short to an integer argument, it may be
    necessary to #ifdef around this on those platforms.

Security implications:
    None known.

Other notes:
    sshconnect.c line 59 (ssh_proxy_connect()) *may* suffer from the
    same bug:

	snprintf(strport, sizeof strport, "%hu", port);

Please let me know if you require further details.

Thanks,

--                          People shouldn't think that it's better to have
Dan Astoorian               loved and lost than never loved at all.  It's
Sysadmin, CSLab             not, it's better to have loved and won.  All
djast at cs.toronto.edu        the other options really suck.    --Dan Redican