From f7430027b41d5ad6220e962a179c2a5213330a44 Mon Sep 17 00:00:00 2001 From: Philipp Rudo Date: Tue, 11 Feb 2020 16:12:21 +0100 Subject: [PATCH] zipl/libc: Replace sprintf with snprintf MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The use of sprintf can easily result in buffer overflows as it assumes that the buffer it writes to is large enough to contain the formatted string. Thus replace sprintf by snprintf and update its users. This removes the last user of vsprintf. Thus also remove vsprintf and its dependencies. Signed-off-by: Philipp Rudo Reviewed-by: Marc Hartmayer Reviewed-by: Stefan Haberland Signed-off-by: Jan Höppner --- zipl/boot/libc.c | 132 +----------------------------------------- zipl/boot/libc.h | 3 +- zipl/boot/menu.c | 2 +- zipl/boot/tape2dump.c | 2 +- 4 files changed, 6 insertions(+), 133 deletions(-) diff --git a/zipl/boot/libc.c b/zipl/boot/libc.c index 0cbb2356..004a0a55 100644 --- a/zipl/boot/libc.c +++ b/zipl/boot/libc.c @@ -127,81 +127,6 @@ int strncmp(const char *s1, const char *s2, unsigned long count) return 0; } -/* - * Convert number to string - * - * Parameters: - * - * - buf: Output buffer - * - base: Base used for formatting (e.g. 10 or 16) - * - val: Number to format - * - zero: If > 0, fill with leading zeros, otherwise use blanks - * - count: Minimum number of characters used for output string - */ -static int num_to_str(char *buf, int base, unsigned long val, int zero, - unsigned long count) -{ - static const char conv_vec[] = {'0', '1', '2', '3', '4', '5', '6', '7', - '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'}; - unsigned long num = 0, val_work = val, in_number = 1; - int i; - - /* Count number of characters needed for number */ - do { - num++; - val_work /= base; - } while (val_work); - /* Real character number overwrites count */ - if (count < num) - count = num; - /* Format number */ - for (i = count - 1; i >= 0; i--) { - if (in_number) { - buf[i] = conv_vec[val % base]; - val /= base; - in_number = val ? 1 : 0; - } else { - buf[i] = zero ? '0' : ' '; - } - } - buf[count] = 0; - return count; -} - -/* - * Convert string to string with indentation - */ -static int str_to_str(char *buf, const char *str, unsigned long count) -{ - unsigned long size; - - size = strlen(str); - if (count < size) - count = size; - else - memset(buf, ' ', count - size); - strcpy(buf + (count - size), str); - return count; -} - -/* - * Convert string to number with given base - */ -unsigned long strtoul(const char *nptr, char **endptr, int base) -{ - unsigned long val = 0; - - while (isdigit(*nptr)) { - if (val != 0) - val *= base; - val += *nptr - '0'; - nptr++; - } - if (endptr) - *endptr = (char *) nptr; - return val; -} - /* * Convert ebcdic string to number with given base */ @@ -469,65 +394,14 @@ static int vsnprintf(char *buf, unsigned long size, const char *fmt, } /* - * Convert string to number with given base + * Write formatted string to buffer */ -static int sprintf_fmt(char type, char *buf, unsigned long val, int zero, - int count) -{ - switch (type) { - case 's': - return str_to_str(buf, (const char *) val, count); - case 'x': - return num_to_str(buf, 16, val, zero, count); - case 'u': - return num_to_str(buf, 10, val, zero, count); - default: - libc_stop(EINTERNAL); - } - return 0; -} - -/* - * Print formated string (va version) - */ -static void vsprintf(char *str, const char *fmt, va_list va) -{ - unsigned long val, zero, count; - char *fmt_next; - - do { - if (*fmt == '%') { - fmt++; - if (*fmt == '0') { - zero = 1; - fmt++; - } else { - zero = 0; - } - /* No number found by strtoul: count=0 fmt_next=fmt */ - count = strtoul(fmt, &fmt_next, 10); - fmt = fmt_next; - if (*fmt == 'l') - fmt++; - val = va_arg(va, unsigned long); - str += sprintf_fmt(*fmt, str, val, zero, count); - fmt++; - } else { - *str++ = *fmt++; - } - } while (*fmt); - *str = 0; -} - -/* - * Write formated string to string - */ -void sprintf(char *str, const char *fmt, ...) +void snprintf(char *buf, unsigned long size, const char *fmt, ...) { va_list va; va_start(va, fmt); - vsprintf(str, fmt, va); + vsnprintf(buf, size, fmt, va); va_end(va); } diff --git a/zipl/boot/libc.h b/zipl/boot/libc.h index 7c7c549e..8b0639ed 100644 --- a/zipl/boot/libc.h +++ b/zipl/boot/libc.h @@ -44,13 +44,12 @@ #define MIB (1024ULL * 1024) void printf(const char *, ...); -void sprintf(char *, const char *, ...); +void snprintf(char *buf, unsigned long size, const char *fmt, ...); void *memcpy(void *, const void *, unsigned long); void *memmove(void *, const void *, unsigned long); void *memset(void *, int c, unsigned long); char *strcat(char *, const char *); int strncmp(const char *, const char *, unsigned long); -unsigned long strtoul(const char *, char **, int); unsigned long ebcstrtoul(char *, char **, int); int strlen(const char *); char *strcpy(char *, const char *); diff --git a/zipl/boot/menu.c b/zipl/boot/menu.c index 2f98a8f7..d36350ef 100644 --- a/zipl/boot/menu.c +++ b/zipl/boot/menu.c @@ -183,7 +183,7 @@ boot: (void *)&__stage2_params + TEXT_OFFSET)); /* append 'BOOT_IMAGE=' to parmline */ - sprintf(endstring, " BOOT_IMAGE=%u", value); + snprintf(endstring, sizeof(endstring), " BOOT_IMAGE=%u", value); if ((strlen(cmd_line_extra) + strlen(endstring)) < COMMAND_LINE_SIZE) strcat(cmd_line_extra, endstring); diff --git a/zipl/boot/tape2dump.c b/zipl/boot/tape2dump.c index 6a1b5c69..f38c52e4 100644 --- a/zipl/boot/tape2dump.c +++ b/zipl/boot/tape2dump.c @@ -188,7 +188,7 @@ static void progress_print_disp(unsigned long addr) if (addr % (1024 * 1024 * 16) != 0) return; - sprintf(msg, "%08u", addr >> 20); + snprintf(msg, sizeof(msg), "%08u", addr >> 20); ccw_load_display(msg); }