[src/trunk]: src/libexec/tftpd Prevent buffer overflows; reported by Maxime V...

[christos <christos%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/bd858512915d
branches:  trunk
changeset: 787703:bd858512915d
user:      christos <christos%NetBSD.org@localhost>
date:      Fri Jun 28 17:20:15 2013 +0000

description:
Prevent buffer overflows; reported by Maxime Villard

diffstat:

 libexec/tftpd/tftpd.c |  60 +++++++++++++++++++++++++++-----------------------
 1 files changed, 32 insertions(+), 28 deletions(-)

diffs (149 lines):

diff -r efa2dfab8450 -r bd858512915d libexec/tftpd/tftpd.c
--- a/libexec/tftpd/tftpd.c     Fri Jun 28 17:13:34 2013 +0000
+++ b/libexec/tftpd/tftpd.c     Fri Jun 28 17:20:15 2013 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: tftpd.c,v 1.39 2011/08/29 20:41:07 joerg Exp $ */
+/*     $NetBSD: tftpd.c,v 1.40 2013/06/28 17:20:15 christos Exp $      */
 
 /*
  * Copyright (c) 1983, 1993
@@ -36,7 +36,7 @@
 #if 0
 static char sccsid[] = "@(#)tftpd.c    8.1 (Berkeley) 6/4/93";
 #else
-__RCSID("$NetBSD: tftpd.c,v 1.39 2011/08/29 20:41:07 joerg Exp $");
+__RCSID("$NetBSD: tftpd.c,v 1.40 2013/06/28 17:20:15 christos Exp $");
 #endif
 #endif /* not lint */
 
@@ -419,8 +419,8 @@
 }
 
 static int
-blk_handler(struct tftphdr *tp, char *opt, char *val, char *ack,
-           int *ackl, int *ec)
+blk_handler(struct tftphdr *tp, const char *val, char *ack, size_t asize,
+    size_t *ackl, int *ec)
 {
        unsigned long bsize;
        char *endp;
@@ -450,17 +450,18 @@
        }
 
        tftp_blksize = bsize;
-       strcpy(ack + *ackl, "blksize");
-       *ackl += 8;
-       l = sprintf(ack + *ackl, "%lu", bsize);
-       *ackl += l + 1;
+       if (asize > *ackl && (l = snprintf(ack + *ackl, asize - *ackl,
+           "blksize%c%lu%c", 0, bsize, 0)) > 0)
+               *ackl += l;
+       else
+               return -1;
 
        return 0;
 }
 
 static int
-timeout_handler(struct tftphdr *tp, char *opt, char *val, char *ack,
-               int *ackl, int *ec)
+timeout_handler(struct tftphdr *tp, const char *val, char *ack, size_t asize,
+               size_t *ackl, int *ec)
 {
        unsigned long tout;
        char *endp;
@@ -486,11 +487,11 @@
        }
 
        rexmtval = tout;
-       strcpy(ack + *ackl, "timeout");
-       *ackl += 8;
-       l = sprintf(ack + *ackl, "%lu", tout);
-       *ackl += l + 1;
-
+       if (asize > *ackl && (l = snprintf(ack + *ackl, asize - *ackl,
+           "timeout%c%lu%c", 0, tout, 0)))
+               *ackl += l;
+       else
+               return -1;
        /*
         * Arbitrarily pick a maximum timeout on a request to 3
         * retransmissions if the interval timeout is more than
@@ -507,8 +508,8 @@
 }
 
 static int
-tsize_handler(struct tftphdr *tp, char *opt, char *val, char *ack,
-             int *ackl, int *ec)
+tsize_handler(struct tftphdr *tp, const char *val, char *ack, size_t asize,
+    size_t *ackl, int *ec)
 {
        unsigned long fsize;
        char *endp;
@@ -550,8 +551,8 @@
 
 static const struct tftp_options {
        const char *o_name;
-       int (*o_handler)(struct tftphdr *, char *, char *, char *,
-                        int *, int *);
+       int (*o_handler)(struct tftphdr *, const char *, char *, size_t,
+                        size_t *, int *);
 } options[] = {
        { "blksize", blk_handler },
        { "timeout", timeout_handler },
@@ -564,8 +565,8 @@
  * recognize in oackbuf.
  */
 static int
-get_options(struct tftphdr *tp, char *cp, int size, char *ackb,
-    int *alen, int *err)
+get_options(struct tftphdr *tp, char *cp, int size, char *ackb, size_t asize,
+    size_t *alen, int *err)
 {
        const struct tftp_options *op;
        char *option, *value, *endp;
@@ -597,7 +598,7 @@
                                break;
                }
                if (op->o_name) {
-                       r = op->o_handler(tp, option, value, ackb, alen, &ec);
+                       r = op->o_handler(tp, value, ackb, asize, alen, &ec);
                        if (r < 0) {
                                rv = -1;
                                break;
@@ -621,7 +622,8 @@
        struct formats *pf;
        char    *cp;
        char    *filename, *mode;
-       int      first, ecode, alen, etftp = 0, r;
+       int      first, ecode, etftp = 0, r;
+       size_t alen;
 
        ecode = 0;      /* XXX gcc */
        first = 1;
@@ -666,7 +668,8 @@
        size -= (++cp - (char *) tp);
        if (size > 0 && *cp) {
                alen = 2; /* Skip over opcode */
-               r = get_options(tp, cp, size, oackbuf, &alen, &ecode);
+               r = get_options(tp, cp, size, oackbuf, sizeof(oackbuf),
+                   &alen, &ecode);
                if (r > 0) {
                        etftp = 1;
                } else if (r < 0) {
@@ -708,10 +711,11 @@
                if (tftp_opt_tsize) {
                        int l;
 
-                       strcpy(oackbuf + alen, "tsize");
-                       alen += 6;
-                       l = sprintf(oackbuf + alen, "%u", tftp_tsize);
-                       alen += l + 1;
+                       if (sizeof(oackbuf) > alen &&
+                           (l = snprintf(oackbuf + alen,
+                           sizeof(oackbuf) - alen, "tsize%c%u%c", 0,
+                           tftp_tsize, 0)))
+                               alen += l;
                }
                oack_h = (struct tftphdr *) oackbuf;
                oack_h->th_opcode = htons(OACK);