git: b15e052e7415 - main - tftpd: Plug memory leaks in option handling code.
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Fri, 18 Nov 2022 16:00:53 UTC
The branch main has been updated by des:
URL: https://cgit.FreeBSD.org/src/commit/?id=b15e052e7415266b18bae08245109ee5dc7a4681
commit b15e052e7415266b18bae08245109ee5dc7a4681
Author: Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2022-11-18 15:39:15 +0000
Commit: Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2022-11-18 15:39:44 +0000
tftpd: Plug memory leaks in option handling code.
Sponsored by: Klara, Inc.
Differential Revision: https://reviews.freebsd.org/D37423
---
libexec/tftpd/tftp-io.c | 2 +-
libexec/tftpd/tftp-options.c | 79 ++++++++++++++++++++++++++++++++++++--------
libexec/tftpd/tftp-options.h | 5 +++
usr.bin/tftp/main.c | 26 ++++++---------
4 files changed, 82 insertions(+), 30 deletions(-)
diff --git a/libexec/tftpd/tftp-io.c b/libexec/tftpd/tftp-io.c
index 245d73ffb871..4504946e910f 100644
--- a/libexec/tftpd/tftp-io.c
+++ b/libexec/tftpd/tftp-io.c
@@ -258,7 +258,7 @@ send_rrq(int peer, char *filename, char *mode)
size += strlen(mode) + 1;
if (options_rfc_enabled) {
- options[OPT_TSIZE].o_request = strdup("0");
+ options_set_request(OPT_TSIZE, "0");
size += make_options(peer, bp, sizeof(buf) - size);
}
diff --git a/libexec/tftpd/tftp-options.c b/libexec/tftpd/tftp-options.c
index 01c47e66e9a5..cc902c7d2110 100644
--- a/libexec/tftpd/tftp-options.c
+++ b/libexec/tftpd/tftp-options.c
@@ -37,6 +37,7 @@ __FBSDID("$FreeBSD$");
#include <arpa/tftp.h>
#include <ctype.h>
+#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -64,6 +65,62 @@ struct options options[] = {
int options_rfc_enabled = 1;
int options_extra_enabled = 1;
+int
+options_set_request(enum opt_enum opt, const char *fmt, ...)
+{
+ va_list ap;
+ char *str;
+ int ret;
+
+ if (fmt == NULL) {
+ str = NULL;
+ } else {
+ va_start(ap, fmt);
+ ret = vasprintf(&str, fmt, ap);
+ va_end(ap);
+ if (ret < 0)
+ return (ret);
+ }
+ if (options[opt].o_request != NULL &&
+ options[opt].o_request != options[opt].o_reply)
+ free(options[opt].o_request);
+ options[opt].o_request = str;
+ return (0);
+}
+
+int
+options_set_reply(enum opt_enum opt, const char *fmt, ...)
+{
+ va_list ap;
+ char *str;
+ int ret;
+
+ if (fmt == NULL) {
+ str = NULL;
+ } else {
+ va_start(ap, fmt);
+ ret = vasprintf(&str, fmt, ap);
+ va_end(ap);
+ if (ret < 0)
+ return (ret);
+ }
+ if (options[opt].o_reply != NULL &&
+ options[opt].o_reply != options[opt].o_request)
+ free(options[opt].o_reply);
+ options[opt].o_reply = str;
+ return (0);
+}
+
+static void
+options_set_reply_equal_request(enum opt_enum opt)
+{
+
+ if (options[opt].o_reply != NULL &&
+ options[opt].o_reply != options[opt].o_request)
+ free(options[opt].o_reply);
+ options[opt].o_reply = options[opt].o_request;
+}
+
/*
* Rules for the option handlers:
* - If there is no o_request, there will be no processing.
@@ -90,12 +147,10 @@ option_tsize(int peer __unused, struct tftphdr *tp __unused, int mode,
return (0);
if (mode == RRQ)
- asprintf(&options[OPT_TSIZE].o_reply,
- "%ju", (uintmax_t)stbuf->st_size);
+ options_set_reply(OPT_TSIZE, "%ju", stbuf->st_size);
else
/* XXX Allows writes of all sizes. */
- options[OPT_TSIZE].o_reply =
- strdup(options[OPT_TSIZE].o_request);
+ options_set_reply_equal_request(OPT_TSIZE);
return (0);
}
@@ -119,8 +174,7 @@ option_timeout(int peer)
exit(1);
} else {
timeoutpacket = to;
- options[OPT_TIMEOUT].o_reply =
- strdup(options[OPT_TIMEOUT].o_request);
+ options_set_reply_equal_request(OPT_TIMEOUT);
}
settimeouts(timeoutpacket, timeoutnetwork, maxtimeouts);
@@ -151,8 +205,7 @@ option_rollover(int peer)
}
return (0);
}
- options[OPT_ROLLOVER].o_reply =
- strdup(options[OPT_ROLLOVER].o_request);
+ options_set_reply_equal_request(OPT_ROLLOVER);
if (debug & DEBUG_OPTIONS)
tftp_log(LOG_DEBUG, "Setting rollover to '%s'",
@@ -212,7 +265,7 @@ option_blksize(int peer)
}
}
- asprintf(&options[OPT_BLKSIZE].o_reply, "%d", size);
+ options_set_reply(OPT_BLKSIZE, "%d", size);
segsize = size;
pktsize = size + 4;
if (debug & DEBUG_OPTIONS)
@@ -266,7 +319,7 @@ option_blksize2(int peer __unused)
/* No need to return */
}
- asprintf(&options[OPT_BLKSIZE2].o_reply, "%d", size);
+ options_set_reply(OPT_BLKSIZE2, "%d", size);
segsize = size;
pktsize = size + 4;
if (debug & DEBUG_OPTIONS)
@@ -301,7 +354,7 @@ option_windowsize(int peer)
}
/* XXX: Should force a windowsize of 1 for non-seekable files. */
- asprintf(&options[OPT_WINDOWSIZE].o_reply, "%d", size);
+ options_set_reply(OPT_WINDOWSIZE, "%d", size);
windowsize = size;
if (debug & DEBUG_OPTIONS)
@@ -391,7 +444,7 @@ parse_options(int peer, char *buffer, uint16_t size)
for (i = 0; options[i].o_type != NULL; i++) {
if (strcmp(option, options[i].o_type) == 0) {
if (!acting_as_client)
- options[i].o_request = value;
+ options_set_request(i, "%s", value);
if (!options_extra_enabled && !options[i].rfc) {
tftp_log(LOG_INFO,
"Option '%s' with value '%s' found "
@@ -422,5 +475,5 @@ void
init_options(void)
{
- options[OPT_ROLLOVER].o_request = strdup("0");
+ options_set_request(OPT_ROLLOVER, "0");
}
diff --git a/libexec/tftpd/tftp-options.h b/libexec/tftpd/tftp-options.h
index cb387dba46c0..569d88d3c6d1 100644
--- a/libexec/tftpd/tftp-options.h
+++ b/libexec/tftpd/tftp-options.h
@@ -64,3 +64,8 @@ enum opt_enum {
OPT_ROLLOVER,
OPT_WINDOWSIZE,
};
+
+int options_set_request(enum opt_enum, const char *, ...)
+ __printflike(2, 3);
+int options_set_reply(enum opt_enum, const char *, ...)
+ __printflike(2, 3);
diff --git a/usr.bin/tftp/main.c b/usr.bin/tftp/main.c
index b6d7bd2a0dcc..cfd486fb8418 100644
--- a/usr.bin/tftp/main.c
+++ b/usr.bin/tftp/main.c
@@ -496,7 +496,7 @@ put(int argc, char *argv[])
close(fd);
return;
}
- asprintf(&options[OPT_TSIZE].o_request, "%ju", sb.st_size);
+ options_set_request(OPT_TSIZE, "%ju", (uintmax_t)sb.st_size);
if (verbose)
printf("putting %s to %s:%s [%s]\n",
@@ -524,7 +524,7 @@ put(int argc, char *argv[])
free(path);
continue;
}
- asprintf(&options[OPT_TSIZE].o_request, "%ju", sb.st_size);
+ options_set_request(OPT_TSIZE, "%ju", (uintmax_t)sb.st_size);
if (verbose)
printf("putting %s to %s:%s [%s]\n",
@@ -926,16 +926,13 @@ setrollover(int argc, char *argv[])
if (argc == 2) {
if (strcasecmp(argv[1], "never") == 0 ||
strcasecmp(argv[1], "none") == 0) {
- free(options[OPT_ROLLOVER].o_request);
- options[OPT_ROLLOVER].o_request = NULL;
+ options_set_request(OPT_ROLLOVER, NULL);
}
if (strcasecmp(argv[1], "1") == 0) {
- free(options[OPT_ROLLOVER].o_request);
- options[OPT_ROLLOVER].o_request = strdup("1");
+ options_set_request(OPT_ROLLOVER, "1");
}
if (strcasecmp(argv[1], "0") == 0) {
- free(options[OPT_ROLLOVER].o_request);
- options[OPT_ROLLOVER].o_request = strdup("0");
+ options_set_request(OPT_ROLLOVER, "0");
}
}
printf("Support for the rollover options is %s.\n",
@@ -1001,10 +998,9 @@ setblocksize(int argc, char *argv[])
printf("Blocksize can't be bigger than %ld bytes due "
"to the net.inet.udp.maxdgram sysctl limitation.\n",
maxdgram - 4);
- asprintf(&options[OPT_BLKSIZE].o_request,
- "%ld", maxdgram - 4);
+ options_set_request(OPT_BLKSIZE, "%ld", maxdgram - 4);
} else {
- asprintf(&options[OPT_BLKSIZE].o_request, "%d", size);
+ options_set_request(OPT_BLKSIZE, "%d", size);
}
}
printf("Blocksize is now %s bytes.\n", options[OPT_BLKSIZE].o_request);
@@ -1057,10 +1053,9 @@ setblocksize2(int argc, char *argv[])
for (i = 0; sizes[i+1] != 0; i++) {
if ((int)maxdgram < sizes[i+1]) break;
}
- asprintf(&options[OPT_BLKSIZE2].o_request,
- "%d", sizes[i]);
+ options_set_request(OPT_BLKSIZE2, "%d", sizes[i]);
} else {
- asprintf(&options[OPT_BLKSIZE2].o_request, "%d", size);
+ options_set_request(OPT_BLKSIZE2, "%d", size);
}
}
printf("Blocksize2 is now %s bytes.\n",
@@ -1094,8 +1089,7 @@ setwindowsize(int argc, char *argv[])
"blocks.\n", WINDOWSIZE_MIN, WINDOWSIZE_MAX);
return;
} else {
- asprintf(&options[OPT_WINDOWSIZE].o_request, "%d",
- size);
+ options_set_request(OPT_WINDOWSIZE, "%d", size);
}
}
printf("Windowsize is now %s blocks.\n",