From owner-misc@openbsd.org Sun Nov 1 17:32:53 1998 Received: from openbsd.cs.colorado.edu (openbsd.cs.colorado.edu [128.138.192.83]) by peecee.gregor.com (8.8.7/8.8.8) with ESMTP id RAA21002 for ; Sun, 1 Nov 1998 17:32:49 -0500 (EST) Received: from localhost (daemon@localhost) by openbsd.cs.colorado.edu (8.9.1a/8.9.1) with SMTP id QAA13726; Sun, 1 Nov 1998 16:06:38 -0700 (MST) Received: by openbsd.cs.colorado.edu (bulk_mailer v1.9); Sun, 1 Nov 1998 16:03:21 -0700 Received: (from domo@localhost) by openbsd.cs.colorado.edu (8.9.1a/8.9.1) id QAA06039 for misc-list; Sun, 1 Nov 1998 16:03:20 -0700 (MST) Received: from naughty.monkey.org (naughty.monkey.org [152.160.231.194]) by openbsd.cs.colorado.edu (8.9.1a/8.9.1) with ESMTP id QAA17670 for ; Sun, 1 Nov 1998 16:03:10 -0700 (MST) Received: by naughty.monkey.org (VMailer, from userid 1001) id CEC814B9F; Sun, 1 Nov 1998 17:28:52 -0500 (EST) Date: Sun, 1 Nov 1998 17:28:51 -0500 (EST) From: Dug Song To: misc@openbsd.org Subject: ssh-1.2.26 patch for log_msg() overflow Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-misc@openbsd.org (i already posted this to bugtraq, but i thought this was important enough to merit a separate message now to the OpenBSD community.) here is a patch for ssh-1.2.26 to fix the overflow in log_msg(), as detailed in IBM's emergency response service bulletin ERS-SVA-E01-1998:005.1 (included in rootshell security bulletin #25). -d. --- http://www.monkey.org/~dugsong/ diff -u -r ssh-1.2.26-orig/Makefile.in ssh-1.2.26/Makefile.in --- ssh-1.2.26-orig/Makefile.in Wed Jul 8 12:40:39 1998 +++ ssh-1.2.26/Makefile.in Sun Nov 1 16:11:44 1998 @@ -315,7 +315,7 @@ rsa.o randoms.o md5.o buffer.o emulate.o packet.o compress.o \ xmalloc.o ttymodes.o newchannels.o bufaux.o authfd.o authfile.o \ crc32.o rsaglue.o cipher.o des.o match.o arcfour.o mpaux.o \ - userfile.o signals.o blowfish.o deattack.o + userfile.o signals.o blowfish.o deattack.o snprintf.o SSHD_OBJS = sshd.o auth-rhosts.o auth-passwd.o auth-rsa.o auth-rh-rsa.o pty.o \ log-server.o login.o hostfile.o canohost.o servconf.o tildexpand.o \ serverloop.o $(COMMON_OBJS) $(KERBEROS_OBJS) $(SSHDCONFOBJS) @@ -332,7 +332,7 @@ xmalloc.o bufaux.o authfd.o authfile.o cipher.o blowfish.o \ des.o arcfour.o mpaux.o userfile.o signals.o $(LIBOBJS) \ $(CONFOBJS) -SCP_OBJS = scp.o xmalloc.o +SCP_OBJS = scp.o xmalloc.o snprintf.o #ifdef F_SECURE_COMMERCIAL # # @@ -359,7 +359,7 @@ randoms.h ttymodes.h authfd.h crc32.h includes.h \ readconf.h userfile.h blowfish.h des.h md5.h rsa.h version.h bufaux.h \ mpaux.h servconf.h xmalloc.h buffer.h emulate.h packet.h ssh.h \ - deattack.h + deattack.h snprintf.h DISTFILES = $(srcdir)/COPYING $(srcdir)/README $(srcdir)/README.SECURID \ $(srcdir)/README.TIS $(srcdir)/README.SECURERPC \ diff -u -r ssh-1.2.26-orig/log-server.c ssh-1.2.26/log-server.c --- ssh-1.2.26-orig/log-server.c Wed Jul 8 12:40:36 1998 +++ ssh-1.2.26/log-server.c Sun Nov 1 16:14:23 1998 @@ -60,6 +60,7 @@ #include "packet.h" #include "xmalloc.h" #include "ssh.h" +#include "snprintf.h" static int log_debug = 0; static int log_quiet = 0; @@ -134,7 +135,7 @@ if (log_quiet) return; va_start(args, fmt); - vsprintf(buf, fmt, args); + vsnprintf(buf, sizeof(buf), fmt, args); va_end(args); if (log_on_stderr) fprintf(stderr, "log: %s\n", buf); @@ -175,7 +176,7 @@ if (log_quiet) return; va_start(args, fmt); - vsprintf(buf, fmt, args); + vsnprintf(buf, sizeof(buf), fmt, args); va_end(args); if (log_on_stderr) fprintf(stderr, "log: %s\n", buf); @@ -191,7 +192,7 @@ if (!log_debug || log_quiet) return; va_start(args, fmt); - vsprintf(buf, fmt, args); + vsnprintf(buf, sizeof(buf), fmt, args); va_end(args); if (log_on_stderr) fprintf(stderr, "debug: %s\n", buf); @@ -207,7 +208,7 @@ if (log_quiet) return; va_start(args, fmt); - vsprintf(buf, fmt, args); + vsnprintf(buf, sizeof(buf), fmt, args); va_end(args); if (log_on_stderr) fprintf(stderr, "error: %s\n", buf); @@ -302,7 +303,7 @@ if (log_quiet) exit(1); va_start(args, fmt); - vsprintf(buf, fmt, args); + vsnprintf(buf, sizeof(buf), fmt, args); va_end(args); if (log_on_stderr) fprintf(stderr, "fatal: %s\n", buf); @@ -321,7 +322,7 @@ if (log_quiet) exit(1); va_start(args, fmt); - vsprintf(buf, fmt, args); + vsnprintf(buf, sizeof(buf), fmt, args); va_end(args); if (log_on_stderr) fprintf(stderr, "fatal: %s\n", buf); diff -u -r ssh-1.2.26-orig/packet.c ssh-1.2.26/packet.c --- ssh-1.2.26-orig/packet.c Wed Jul 8 12:40:37 1998 +++ ssh-1.2.26/packet.c Sun Nov 1 16:15:26 1998 @@ -90,6 +90,7 @@ #include "getput.h" #include "compress.h" #include "deattack.h" +#include "snprintf.h" /* This variable contains the file descriptors used for communicating with the other side. connection_in is used for reading; connection_out @@ -693,7 +694,7 @@ va_list args; va_start(args, fmt); - vsprintf(buf, fmt, args); + vsnprintf(buf, sizeof(buf), fmt, args); va_end(args); packet_start(SSH_MSG_DEBUG); @@ -719,7 +720,7 @@ /* Format the message. Note that the caller must make sure the message is of limited size. */ va_start(args, fmt); - vsprintf(buf, fmt, args); + vsnprintf(buf, sizeof(buf), fmt, args); va_end(args); /* Send the disconnect message to the other side, and wait for it to get diff -u -r ssh-1.2.26-orig/scp.c ssh-1.2.26/scp.c --- ssh-1.2.26-orig/scp.c Wed Jul 8 12:40:38 1998 +++ ssh-1.2.26/scp.c Sun Nov 1 16:34:57 1998 @@ -134,6 +134,7 @@ #include "includes.h" #include "ssh.h" #include "xmalloc.h" +#include "snprintf.h" #ifdef HAVE_UTIME_H #include #if defined(_NEXT_SOURCE) && !defined(_POSIX_SOURCE) @@ -332,7 +333,7 @@ char buf[1024]; va_start(ap, fmt); - vsprintf(buf, fmt, ap); + vsnprintf(buf, sizeof(buf), fmt, ap); va_end(ap); fprintf(stderr, "%s\n", buf); exit(255); diff -u -r ssh-1.2.26-orig/snprintf.c ssh-1.2.26/snprintf.c --- ssh-1.2.26-orig/snprintf.c Sun Nov 1 16:19:33 1998 +++ ssh-1.2.26/snprintf.c Sun Nov 1 16:24:37 1998 @@ -0,0 +1,559 @@ +/* + + Author: Tomi Salo + + Copyright (C) 1996 SSH Communications Security Oy, Espoo, Finland + All rights reserved. + + Implementation of functions snprintf() and vsnprintf() + + */ + +/* + * $Id: snprintf.c,v 1.19 1998/06/03 00:45:30 ylo Exp $ + * $Log: snprintf.c,v $ + * $EndLog$ + */ + +#include "includes.h" +#include "snprintf.h" + +#define MINUS_FLAG 0x1 +#define PLUS_FLAG 0x2 +#define SPACE_FLAG 0x4 +#define HASH_FLAG 0x8 +#define CONV_TO_SHORT 0x10 +#define IS_LONG_INT 0x20 +#define IS_LONG_DOUBLE 0x40 +#define X_UPCASE 0x80 +#define IS_NEGATIVE 0x100 +#define UNSIGNED_DEC 0x200 +#define ZERO_PADDING 0x400 + +#undef sprintf + +/* Extract a formatting directive from str. Str must point to a '%'. + Returns number of characters used or zero if extraction failed. */ + +int +snprintf_get_directive(const char *str, int *flags, int *width, + int *precision, char *format_char, va_list *ap) +{ + int length, n; + const char *orig_str = str; + + *flags = 0; + *width = 0; + *precision = 0; + *format_char = (char)0; + + if (*str == '%') + { + /* Get the flags */ + str++; + while (*str == '-' || *str == '+' || *str == ' ' + || *str == '#' || *str == '0') + { + switch (*str) + { + case '-': + *flags |= MINUS_FLAG; + break; + case '+': + *flags |= PLUS_FLAG; + break; + case ' ': + *flags |= SPACE_FLAG; + break; + case '#': + *flags |= HASH_FLAG; + break; + case '0': + *flags |= ZERO_PADDING; + break; + } + str++; + } + + /* Don't pad left-justified numbers withs zeros */ + if ((*flags & MINUS_FLAG) && (*flags & ZERO_PADDING)) + *flags &= ~ZERO_PADDING; + + /* Is width field present? */ + if (isdigit(*str)) + { + n = sscanf(str, "%d", width); + if (n == 0) + return 0; + + /* Step through the field */ + while (isdigit(*str)) + str++; + } + else + if (*str == '*') + { + *width = va_arg(*ap, int); + str++; + } + + /* Is the precision field present? */ + if (*str == '.') + { + str++; + if (isdigit(*str)) + { + n = sscanf(str, "%d", precision); + if (n == 0) + return 0; + + /* Step through the field */ + while (isdigit(*str)) + str++; + } + else + if (*str == '*') + { + *precision = va_arg(*ap, int); + str++; + } + else + *precision = 0; + } + + /* Get the optional type character */ + if (*str == 'h') + { + *flags |= CONV_TO_SHORT; + str++; + } + else + { + if (*str == 'l') + { + *flags |= IS_LONG_INT; + str++; + } + else + { + if (*str == 'L') + { + *flags |= IS_LONG_DOUBLE; + str++; + } + } + } + + /* Get and check the formatting character */ + + *format_char = *str; + str++; + length = str - orig_str; + + switch (*format_char) + { + case 'i': case 'd': case 'o': case 'u': case 'x': case 'X': + case 'f': case 'e': case 'E': case 'g': case 'G': + case 'c': case 's': case 'p': case 'n': + if (*format_char == 'X') + *flags |= X_UPCASE; + if (*format_char == 'o') + *flags |= UNSIGNED_DEC; + return length; + + default: + return 0; + } + } + else + { + return 0; + } +} + +/* Convert a integer from unsigned long int representation + to string representation. This will insert prefixes if needed + (leading zero for octal and 0x or 0X for hexadecimal) and + will write at most buf_size characters to buffer. + tmp_buf is used because we want to get correctly truncated + results. + */ + +int +snprintf_convert_ulong(char *buffer, size_t buf_size, int base, char *digits, + unsigned long int ulong_val, int flags, int width, + int precision) +{ + int tmp_buf_len = 100 + width, len; + char *tmp_buf, *tmp_buf_ptr, prefix[2]; + tmp_buf = xmalloc(tmp_buf_len); + + prefix[0] = '\0'; + prefix[1] = '\0'; + + /* Make tmp_buf_ptr point just past the last char of buffer */ + tmp_buf_ptr = tmp_buf + tmp_buf_len; + + /* Main conversion loop */ + do + { + *--tmp_buf_ptr = digits[ulong_val % base]; + ulong_val /= base; + precision--; + } + while ((ulong_val != 0 || precision > 0) && tmp_buf_ptr > tmp_buf); + + /* Get the prefix */ + if (!(flags & IS_NEGATIVE)) + { + if (base == 16 && (flags & HASH_FLAG)) + if (flags && X_UPCASE) + { + prefix[0] = 'x'; + prefix[1] = '0'; + } + else + { + prefix[0] = 'X'; + prefix[1] = '0'; + } + + if (base == 8 && (flags & HASH_FLAG)) + prefix[0] = '0'; + + if (base == 10 && !(flags & UNSIGNED_DEC) && (flags & PLUS_FLAG)) + prefix[0] = '+'; + else + if (base == 10 && !(flags & UNSIGNED_DEC) && (flags & SPACE_FLAG)) + prefix[0] = ' '; + } + else + prefix[0] = '-'; + + if (prefix[0] != '\0' && tmp_buf_ptr > tmp_buf) + { + *--tmp_buf_ptr = prefix[0]; + if (prefix[1] != '\0' && tmp_buf_ptr > tmp_buf) + *--tmp_buf_ptr = prefix[1]; + } + + len = (tmp_buf + tmp_buf_len) - tmp_buf_ptr; + + if (len <= buf_size) + { + if (len < width) + { + if (width > (tmp_buf_ptr - tmp_buf)) + width = (tmp_buf_ptr - tmp_buf); + if (flags & MINUS_FLAG) + { + memcpy(buffer, tmp_buf_ptr, len); + memset(buffer + len, (flags & ZERO_PADDING)?'0':' ', + width - len); + len = width; + } + else + { + memset(buffer, (flags & ZERO_PADDING)?'0':' ', + width - len); + memcpy(buffer + width - len, tmp_buf_ptr, len); + len = width; + } + } + else + { + memcpy(buffer, tmp_buf_ptr, len); + } + xfree(tmp_buf); + return len; + } + else + { + memcpy(buffer, tmp_buf_ptr, buf_size); + xfree(tmp_buf); + return buf_size; + } +} + +int +snprintf_convert_float(char *buffer, size_t buf_size, + double dbl_val, int flags, int width, + int precision, char format_char) +{ + char print_buf[160], print_buf_len = 0; + char format_str[80], *format_str_ptr; + + format_str_ptr = format_str; + + if (width > 155) width = 155; + if (precision <= 0) + precision = 6; + if (precision > 120) + precision = 120; + + /* Construct the formatting string and let system's sprintf + do the real work. */ + + *format_str_ptr++ = '%'; + + if (flags & MINUS_FLAG) + *format_str_ptr++ = '-'; + if (flags & PLUS_FLAG) + *format_str_ptr++ = '+'; + if (flags & SPACE_FLAG) + *format_str_ptr++ = ' '; + if (flags & ZERO_PADDING) + *format_str_ptr++ = '0'; + if (flags & HASH_FLAG) + *format_str_ptr++ = '#'; + + format_str_ptr += sprintf(format_str_ptr, "%d.%d", width, precision); + if (flags & IS_LONG_DOUBLE) + *format_str_ptr++ = 'L'; + *format_str_ptr++ = format_char; + *format_str_ptr++ = '\0'; + + print_buf_len = sprintf(print_buf, format_str, dbl_val); + + if (print_buf_len > buf_size) print_buf_len = buf_size; + strncpy(buffer, print_buf, print_buf_len); + return print_buf_len; +} + +int +snprintf(char *str, size_t size, const char *format, ...) +{ + int ret; + va_list ap; + va_start(ap, format); + ret = vsnprintf(str, size, format, ap); + va_end(ap); + + return ret; +} + +int +vsnprintf(char *str, size_t size, const char *format, va_list ap) +{ + int status, left = (int)size - 1; + const char *format_ptr = format; + int flags, width, precision, i; + char format_char, *orig_str = str; + int *int_ptr; + long int long_val; + unsigned long int ulong_val; + char *str_val; + double dbl_val; + + flags = 0; + while (format_ptr < format + strlen(format)) + { + if (*format_ptr == '%') + { + if (format_ptr[1] == '%' && left > 0) + { + *str++ = '%'; + left--; + format_ptr += 2; + } + else + { + if (left <= 0) + { + *str = '\0'; + return size; + } + else + { + status = snprintf_get_directive(format_ptr, &flags, &width, + &precision, &format_char, + &ap); + if (status == 0) + { + *str = '\0'; + return 0; + } + else + { + format_ptr += status; + /* Print a formatted argument */ + switch (format_char) + { + case 'i': case 'd': + /* Convert to unsigned long int before + actual conversion to string */ + if (flags & IS_LONG_INT) + long_val = va_arg(ap, long int); + else + long_val = (long int) va_arg(ap, int); + + if (long_val < 0) + { + ulong_val = (unsigned long int) -long_val; + flags |= IS_NEGATIVE; + } + else + { + ulong_val = (unsigned long int) long_val; + } + status = snprintf_convert_ulong(str, left, 10, + "0123456789", + ulong_val, flags, + width, precision); + str += status; + left -= status; + break; + + case 'x': + if (flags & IS_LONG_INT) + ulong_val = va_arg(ap, unsigned long int); + else + ulong_val = + (unsigned long int) va_arg(ap, unsigned int); + + status = snprintf_convert_ulong(str, left, 16, + "0123456789abcdef", + ulong_val, flags, + width, precision); + str += status; + left -= status; + break; + + case 'X': + if (flags & IS_LONG_INT) + ulong_val = va_arg(ap, unsigned long int); + else + ulong_val = + (unsigned long int) va_arg(ap, unsigned int); + + status = snprintf_convert_ulong(str, left, 16, + "0123456789ABCDEF", + ulong_val, flags, + width, precision); + str += status; + left -= status; + break; + + case 'o': + if (flags & IS_LONG_INT) + ulong_val = va_arg(ap, unsigned long int); + else + ulong_val = + (unsigned long int) va_arg(ap, unsigned int); + + status = snprintf_convert_ulong(str, left, 8, + "01234567", + ulong_val, flags, + width, precision); + str += status; + left -= status; + break; + + case 'u': + if (flags & IS_LONG_INT) + ulong_val = va_arg(ap, unsigned long int); + else + ulong_val = + (unsigned long int) va_arg(ap, unsigned int); + + status = snprintf_convert_ulong(str, left, 10, + "0123456789", + ulong_val, flags, + width, precision); + str += status; + left -= status; + break; + + case 'p': + break; + + case 'c': + if (flags & IS_LONG_INT) + ulong_val = va_arg(ap, unsigned long int); + else + ulong_val = + (unsigned long int) va_arg(ap, unsigned int); + *str++ = (unsigned char)ulong_val; + left--; + break; + + case 's': + str_val = va_arg(ap, char *); + + if (str_val == NULL) + str_val = "(null)"; + + if (precision == 0) + precision = strlen(str_val); + else + { + if (memchr(str_val, 0, precision) != NULL) + precision = strlen(str_val); + } + if (precision > left) + precision = left; + + if (width > left) + width = left; + if (width < precision) + width = precision; + i = width - precision; + + if (flags & MINUS_FLAG) + { + strncpy(str, str_val, precision); + memset(str + precision, + (flags & ZERO_PADDING)?'0':' ', i); + } + else + { + memset(str, (flags & ZERO_PADDING)?'0':' ', i); + strncpy(str + i, str_val, precision); + } + str += width; + left -= width; + break; + + case 'n': + int_ptr = va_arg(ap, int *); + *int_ptr = str - orig_str; + break; + + case 'f': case 'e': case 'E': case 'g': case 'G': + if (flags & IS_LONG_DOUBLE) + dbl_val = (double) va_arg(ap, long double); + else + dbl_val = va_arg(ap, double); + status = + snprintf_convert_float(str, left, dbl_val, flags, + width, precision, + format_char); + str += status; + left -= status; + break; + + default: + break; + } + } + } + } + } + else + { + if (left > 0) + { + *str++ = *format_ptr++; + left--; + } + else + { + *str = '\0'; + return size; + } + } + } + *str = '\0'; + return size - left - 1; +} + diff -u -r ssh-1.2.26-orig/snprintf.h ssh-1.2.26/snprintf.h --- ssh-1.2.26-orig/snprintf.h Sun Nov 1 16:19:25 1998 +++ ssh-1.2.26/snprintf.h Sun Nov 1 16:16:48 1998 @@ -0,0 +1,46 @@ +/* + + Author: Tomi Salo + + Copyright (C) 1996 SSH Communications Security Oy, Espoo, Finland + All rights reserved. + + Header file for snprintf.c + + */ + +/* + * $Id: + * $Log: snprintf.h,v $ + * $EndLog$ + */ + +#ifndef SNPRINTF_H +#define SNPRINTF_H + +#include "includes.h" + +/* Write formatted text to buffer 'str', using format string 'format'. + Returns number of characters written, or negative if error + occurred. SshBuffer's size is given in 'size'. Format string is + understood as defined in ANSI C. + + NOTE: This does NOT work identically with BDS's snprintf. + + Integers: Ansi C says that precision specifies the minimun + number of digits to print. BSD's version however counts the + prefixes (+, -, ' ', '0x', '0X', octal prefix '0'...) as + 'digits'. + + Also, BSD implementation does not permit padding integers + to specified width with zeros on left (in front of the prefixes), + it uses spaces instead, even when Ansi C only forbids padding + with zeros on the right side of numbers. + + */ + +int snprintf(char *str, size_t size, const char *format, ...); + +int vsnprintf(char *str, size_t size, const char *format, va_list ap); + +#endif /* SNPRINTF_H */