[tftp] fixes #714261 - CVE-2011-2199: buffer overflow when setting utimeout option

Jiri Skala jskala at fedoraproject.org
Mon Jun 20 14:12:46 UTC 2011


commit d5be562df625d90b1a6c824f03fc59826a740e36
Author: Jiri Skala <jskala at redhat.com>
Date:   Mon Jun 20 16:12:32 2011 +0200

    fixes #714261 - CVE-2011-2199: buffer overflow when setting utimeout option

 tftp-hpa-0.49-buf_over.patch |  265 ++++++++++++++++++++++++++++++++++++++++++
 tftp.spec                    |    7 +-
 2 files changed, 271 insertions(+), 1 deletions(-)
---
diff --git a/tftp-hpa-0.49-buf_over.patch b/tftp-hpa-0.49-buf_over.patch
new file mode 100644
index 0000000..c40a5d5
--- /dev/null
+++ b/tftp-hpa-0.49-buf_over.patch
@@ -0,0 +1,265 @@
+From f3035c45bc50bb5cac87ca01e7ef6a12485184f8 Mon Sep 17 00:00:00 2001
+From: H. Peter Anvin <hpa at linux.intel.com>
+Date: Fri, 10 Jun 2011 11:47:02 -0700
+Subject: [PATCH] tftpd: simplify option parsing
+
+Simplify the option parsing to make use of the fact that all the
+options we support are integer options.  This fixes a buffer overflow
+in the utimeout option.
+
+Reported-by: Timo Warns <warns at pre-sense.de>
+Signed-off-by: H. Peter Anvin <hpa at linux.intel.com>
+---
+ tftpd/tftpd.c |  154 +++++++++++++++++++++++++++------------------------------
+ 1 files changed, 73 insertions(+), 81 deletions(-)
+
+diff --git a/tftpd/tftpd.c b/tftpd/tftpd.c
+index 4c4d4ed..dd3c982 100644
+--- a/tftpd/tftpd.c
++++ b/tftpd/tftpd.c
+@@ -114,17 +114,17 @@ static struct rule *rewrite_rules = NULL;
+ int tftp(struct tftphdr *, int);
+ static void nak(int, const char *);
+ static void timer(int);
+-static void do_opt(char *, char *, char **);
++static void do_opt(const char *, const char *, char **);
+ 
+-static int set_blksize(char *, char **);
+-static int set_blksize2(char *, char **);
+-static int set_tsize(char *, char **);
+-static int set_timeout(char *, char **);
+-static int set_utimeout(char *, char **);
++static int set_blksize(uintmax_t *);
++static int set_blksize2(uintmax_t *);
++static int set_tsize(uintmax_t *);
++static int set_timeout(uintmax_t *);
++static int set_utimeout(uintmax_t *);
+ 
+ struct options {
+     const char *o_opt;
+-    int (*o_fnc) (char *, char **);
++    int (*o_fnc)(uintmax_t *);
+ } options[] = {
+     {"blksize",  set_blksize},
+     {"blksize2", set_blksize2},
+@@ -1175,48 +1175,38 @@ static int blksize_set;
+ /*
+  * Set a non-standard block size (c.f. RFC2348)
+  */
+-static int set_blksize(char *val, char **ret)
++static int set_blksize(uintmax_t *vp)
+ {
+-    static char b_ret[6];
+-    unsigned int sz;
+-    char *vend;
+-
+-    sz = (unsigned int)strtoul(val, &vend, 10);
++    uintmax_t sz = *vp;
+ 
+-    if (blksize_set || *vend)
++    if (blksize_set)
+         return 0;
+ 
+     if (sz < 8)
+-        return (0);
++        return 0;
+     else if (sz > max_blksize)
+         sz = max_blksize;
+ 
+-    segsize = sz;
+-    sprintf(*ret = b_ret, "%u", sz);
+-
++    *vp = segsize = sz;
+     blksize_set = 1;
+-
+-    return (1);
++    return 1;
+ }
+ 
+ /*
+  * Set a power-of-two block size (nonstandard)
+  */
+-static int set_blksize2(char *val, char **ret)
++static int set_blksize2(uintmax_t *vp)
+ {
+-    static char b_ret[6];
+-    unsigned int sz;
+-    char *vend;
++    uintmax_t sz = *vp;
+ 
+-    sz = (unsigned int)strtoul(val, &vend, 10);
+-
+-    if (blksize_set || *vend)
++    if (blksize_set)
+         return 0;
+ 
+     if (sz < 8)
+         return (0);
+     else if (sz > max_blksize)
+         sz = max_blksize;
++    else
+ 
+     /* Convert to a power of two */
+     if (sz & (sz - 1)) {
+@@ -1227,12 +1217,9 @@ static int set_blksize2(char *val, char **ret)
+         sz = sz1;
+     }
+ 
+-    segsize = sz;
+-    sprintf(*ret = b_ret, "%u", sz);
+-
++    *vp = segsize = sz;
+     blksize_set = 1;
+-
+-    return (1);
++    return 1;
+ }
+ 
+ /*
+@@ -1257,22 +1241,18 @@ static int set_rollover(char *val, char **ret)
+  * For netascii mode, we don't know the size ahead of time;
+  * so reject the option.
+  */
+-static int set_tsize(char *val, char **ret)
++static int set_tsize(uintmax_t *vp)
+ {
+-    static char b_ret[sizeof(uintmax_t) * CHAR_BIT / 3 + 2];
+-    uintmax_t sz;
+-    char *vend;
++    uintmax_t sz = *vp;
+ 
+-    sz = strtoumax(val, &vend, 10);
+-
+-    if (!tsize_ok || *vend)
++    if (!tsize_ok)
+         return 0;
+ 
+     if (sz == 0)
+-        sz = (uintmax_t) tsize;
++        sz = tsize;
+ 
+-    sprintf(*ret = b_ret, "%" PRIuMAX, sz);
+-    return (1);
++    *vp = sz;
++    return 1;
+ }
+ 
+ /*
+@@ -1280,74 +1260,86 @@ static int set_tsize(char *val, char **ret)
+  * to be the (default) retransmission timeout, but being an
+  * integer in seconds it seems a bit limited.
+  */
+-static int set_timeout(char *val, char **ret)
++static int set_timeout(uintmax_t *vp)
+ {
+-    static char b_ret[4];
+-    unsigned long to;
+-    char *vend;
++    uintmax_t to = *vp;
+ 
+-    to = strtoul(val, &vend, 10);
+-
+-    if (to < 1 || to > 255 || *vend)
++    if (to < 1 || to > 255)
+         return 0;
+ 
+     rexmtval = timeout = to * 1000000UL;
+     maxtimeout = rexmtval * TIMEOUT_LIMIT;
+ 
+-    sprintf(*ret = b_ret, "%lu", to);
+-    return (1);
++    return 1;
+ }
+ 
+ /* Similar, but in microseconds.  We allow down to 10 ms. */
+-static int set_utimeout(char *val, char **ret)
++static int set_utimeout(uintmax_t *vp)
+ {
+-    static char b_ret[4];
+-    unsigned long to;
+-    char *vend;
++    uintmax_t to = *vp;
+ 
+-    to = strtoul(val, &vend, 10);
+-
+-    if (to < 10000UL || to > 255000000UL || *vend)
++    if (to < 10000UL || to > 255000000UL)
+         return 0;
+ 
+     rexmtval = timeout = to;
+     maxtimeout = rexmtval * TIMEOUT_LIMIT;
+ 
+-    sprintf(*ret = b_ret, "%lu", to);
+-    return (1);
++    return 1;
+ }
+ 
+ /*
+- * Parse RFC2347 style options
++ * Conservative calculation for the size of a buffer which can hold an
++ * arbitrary integer
+  */
+-static void do_opt(char *opt, char *val, char **ap)
++#define OPTBUFSIZE	(sizeof(uintmax_t) * CHAR_BIT / 3 + 3)
++
++/*
++ * Parse RFC2347 style options; we limit the arguments to positive
++ * integers which matches all our current options.
++ */
++static void do_opt(const char *opt, const char *val, char **ap)
+ {
+     struct options *po;
+-    char *ret;
++    char retbuf[OPTBUFSIZE];
++    char *p = *ap;
++    size_t optlen, retlen;
++    char *vend;
++    uintmax_t v;
+ 
+     /* Global option-parsing variables initialization */
+     blksize_set = 0;
+ 
+-    if (!*opt)
++    if (!*opt || !*val)
+         return;
+ 
++    errno = 0;
++    v = strtoumax(val, &vend, 10);
++    if (*vend || errno == ERANGE)
++	return;
++
+     for (po = options; po->o_opt; po++)
+         if (!strcasecmp(po->o_opt, opt)) {
+-            if (po->o_fnc(val, &ret)) {
+-                if (*ap + strlen(opt) + strlen(ret) + 2 >=
+-                    ackbuf + sizeof(ackbuf)) {
++            if (po->o_fnc(&v)) {
++		optlen = strlen(opt);
++		retlen = sprintf(retbuf, "%"PRIuMAX, v);
++
++                if (p + optlen + retlen + 2 >= ackbuf + sizeof(ackbuf)) {
+                     nak(EOPTNEG, "Insufficient space for options");
+                     exit(0);
+                 }
+-                *ap = strrchr(strcpy(strrchr(strcpy(*ap, opt), '\0') + 1,
+-                                     ret), '\0') + 1;
++		
++		memcpy(p, opt, optlen+1);
++		p += optlen+1;
++		memcpy(p, retbuf, retlen+1);
++		p += retlen+1;
+             } else {
+                 nak(EOPTNEG, "Unsupported option(s) requested");
+                 exit(0);
+             }
+             break;
+         }
+-    return;
++
++    *ap = p;
+ }
+ 
+ #ifdef WITH_REGEX
+-- 
+1.7.4.4
+
diff --git a/tftp.spec b/tftp.spec
index 55d39df..1c18c42 100644
--- a/tftp.spec
+++ b/tftp.spec
@@ -1,7 +1,7 @@
 Summary: The client for the Trivial File Transfer Protocol (TFTP)
 Name: tftp
 Version: 0.49
-Release: 8%{?dist}
+Release: 9%{?dist}
 License: BSD
 Group: Applications/Internet
 Source0: http://www.kernel.org/pub/software/network/tftp/tftp-hpa-%{version}.tar.bz2
@@ -14,6 +14,7 @@ Patch4: tftp-0.49-chk_retcodes.patch
 Patch5: tftp-hpa-0.49-fortify-strcpy-crash.patch
 Patch6: tftp-0.49-cmd_arg.patch
 Patch7: tftp-hpa-0.49-stats.patch
+Patch8: tftp-hpa-0.49-buf_over.patch
 
 BuildRequires: tcp_wrappers-devel readline-devel autoconf
 BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
@@ -49,6 +50,7 @@ enabled unless it is expressly needed.  The TFTP server is run from
 %patch5 -p1 -b .fortify-strcpy-crash
 %patch6 -p1 -b .cmd_arg
 %patch7 -p1 -b .stats
+%patch8 -p1 -b .buf_over
 
 %build
 autoreconf
@@ -94,6 +96,9 @@ rm -rf ${RPM_BUILD_ROOT}
 %{_mandir}/man8/*
 
 %changelog
+* Mon Jun 20 2011 Jiri Skala <jskala at redhat.com> - 0.49-9
+- fixes #714261 - CVE-2011-2199: buffer overflow when setting utimeout option
+
 * Wed Feb 09 2011 Fedora Release Engineering <rel-eng at lists.fedoraproject.org> - 0.49-8
 - Rebuilt for https://fedoraproject.org/wiki/Fedora_15_Mass_Rebuild
 


More information about the scm-commits mailing list