summaryrefslogtreecommitdiff
path: root/ntpd/ntp_control.c
diff options
context:
space:
mode:
Diffstat (limited to 'ntpd/ntp_control.c')
-rw-r--r--ntpd/ntp_control.c221
1 files changed, 191 insertions, 30 deletions
diff --git a/ntpd/ntp_control.c b/ntpd/ntp_control.c
index 2e174d0210ab..e5a567e789d6 100644
--- a/ntpd/ntp_control.c
+++ b/ntpd/ntp_control.c
@@ -75,6 +75,7 @@ static void ctl_putarray (const char *, double *, int);
static void ctl_putsys (int);
static void ctl_putpeer (int, struct peer *);
static void ctl_putfs (const char *, tstamp_t);
+static void ctl_printf (const char *, ...) NTP_PRINTF(1, 2);
#ifdef REFCLOCK
static void ctl_putclock (int, struct refclockstat *, int);
#endif /* REFCLOCK */
@@ -111,6 +112,8 @@ static void unset_trap (struct recvbuf *, int);
static struct ctl_trap *ctlfindtrap(sockaddr_u *,
struct interface *);
+int/*BOOL*/ is_safe_filename(const char * name);
+
static const struct ctl_proc control_codes[] = {
{ CTL_OP_UNSPEC, NOAUTH, control_unspec },
{ CTL_OP_READSTAT, NOAUTH, read_status },
@@ -873,10 +876,66 @@ ctl_error(
CTL_HEADER_LEN);
}
+int/*BOOL*/
+is_safe_filename(const char * name)
+{
+ /* We need a strict validation of filenames we should write: The
+ * daemon might run with special permissions and is remote
+ * controllable, so we better take care what we allow as file
+ * name!
+ *
+ * The first character must be digit or a letter from the ASCII
+ * base plane or a '_' ([_A-Za-z0-9]), the following characters
+ * must be from [-._+A-Za-z0-9].
+ *
+ * We do not trust the character classification much here: Since
+ * the NTP protocol makes no provisions for UTF-8 or local code
+ * pages, we strictly require the 7bit ASCII code page.
+ *
+ * The following table is a packed bit field of 128 two-bit
+ * groups. The LSB in each group tells us if a character is
+ * acceptable at the first position, the MSB if the character is
+ * accepted at any other position.
+ *
+ * This does not ensure that the file name is syntactically
+ * correct (multiple dots will not work with VMS...) but it will
+ * exclude potential globbing bombs and directory traversal. It
+ * also rules out drive selection. (For systems that have this
+ * notion, like Windows or VMS.)
+ */
+ static const uint32_t chclass[8] = {
+ 0x00000000, 0x00000000,
+ 0x28800000, 0x000FFFFF,
+ 0xFFFFFFFC, 0xC03FFFFF,
+ 0xFFFFFFFC, 0x003FFFFF
+ };
+
+ u_int widx, bidx, mask;
+ if (!*name)
+ return FALSE;
+
+ mask = 1u;
+ while (0 != (widx = (u_char)*name++)) {
+ bidx = (widx & 15) << 1;
+ widx = widx >> 4;
+ if (widx >= sizeof(chclass))
+ return FALSE;
+ if (0 == ((chclass[widx] >> bidx) & mask))
+ return FALSE;
+ mask |= 2u;
+ }
+ return TRUE;
+}
+
+
/*
* save_config - Implements ntpq -c "saveconfig <filename>"
* Writes current configuration including any runtime
* changes by ntpq's :config or config-from-file
+ *
+ * Note: There should be no buffer overflow or truncation in the
+ * processing of file names -- both cause security problems. This is bit
+ * painful to code but essential here.
*/
void
save_config(
@@ -904,24 +963,38 @@ save_config(
"\\/" /* separator and critical char for POSIX */
#endif
;
-
-
char reply[128];
#ifdef SAVECONFIG
+ static const char savedconfig_eq[] = "savedconfig=";
+
+ /* Build a safe open mode from the available mode flags. We want
+ * to create a new file and write it in text mode (when
+ * applicable -- only Windows does this...)
+ */
+ static const int openmode = O_CREAT | O_TRUNC | O_WRONLY
+# if defined(O_EXCL) /* posix, vms */
+ | O_EXCL
+# elif defined(_O_EXCL) /* windows is alway very special... */
+ | _O_EXCL
+# endif
+# if defined(_O_TEXT) /* windows, again */
+ | _O_TEXT
+#endif
+ ;
+
char filespec[128];
char filename[128];
char fullpath[512];
- const char savedconfig_eq[] = "savedconfig=";
char savedconfig[sizeof(savedconfig_eq) + sizeof(filename)];
time_t now;
int fd;
FILE *fptr;
+ int prc;
+ size_t reqlen;
#endif
if (RES_NOMODIFY & restrict_mask) {
- snprintf(reply, sizeof(reply),
- "saveconfig prohibited by restrict ... nomodify");
- ctl_putdata(reply, strlen(reply), 0);
+ ctl_printf("%s", "saveconfig prohibited by restrict ... nomodify");
ctl_flushpkt(0);
NLOG(NLOG_SYSINFO)
msyslog(LOG_NOTICE,
@@ -933,9 +1006,7 @@ save_config(
#ifdef SAVECONFIG
if (NULL == saveconfigdir) {
- snprintf(reply, sizeof(reply),
- "saveconfig prohibited, no saveconfigdir configured");
- ctl_putdata(reply, strlen(reply), 0);
+ ctl_printf("%s", "saveconfig prohibited, no saveconfigdir configured");
ctl_flushpkt(0);
NLOG(NLOG_SYSINFO)
msyslog(LOG_NOTICE,
@@ -944,21 +1015,79 @@ save_config(
return;
}
- if (0 == reqend - reqpt)
+ /* The length checking stuff gets serious. Do not assume a NUL
+ * byte can be found, but if so, use it to calculate the needed
+ * buffer size. If the available buffer is too short, bail out;
+ * likewise if there is no file spec. (The latter will not
+ * happen when using NTPQ, but there are other ways to craft a
+ * network packet!)
+ */
+ reqlen = (size_t)(reqend - reqpt);
+ if (0 != reqlen) {
+ char * nulpos = (char*)memchr(reqpt, 0, reqlen);
+ if (NULL != nulpos)
+ reqlen = (size_t)(nulpos - reqpt);
+ }
+ if (0 == reqlen)
return;
+ if (reqlen >= sizeof(filespec)) {
+ ctl_printf("saveconfig exceeded maximum raw name length (%u)",
+ (u_int)sizeof(filespec));
+ ctl_flushpkt(0);
+ msyslog(LOG_NOTICE,
+ "saveconfig exceeded maximum raw name length from %s",
+ stoa(&rbufp->recv_srcadr));
+ return;
+ }
- strlcpy(filespec, reqpt, sizeof(filespec));
- time(&now);
-
+ /* copy data directly as we exactly know the size */
+ memcpy(filespec, reqpt, reqlen);
+ filespec[reqlen] = '\0';
+
/*
* allow timestamping of the saved config filename with
* strftime() format such as:
* ntpq -c "saveconfig ntp-%Y%m%d-%H%M%S.conf"
* XXX: Nice feature, but not too safe.
+ * YYY: The check for permitted characters in file names should
+ * weed out the worst. Let's hope 'strftime()' does not
+ * develop pathological problems.
*/
+ time(&now);
if (0 == strftime(filename, sizeof(filename), filespec,
- localtime(&now)))
+ localtime(&now)))
+ {
+ /*
+ * If we arrive here, 'strftime()' balked; most likely
+ * the buffer was too short. (Or it encounterd an empty
+ * format, or just a format that expands to an empty
+ * string.) We try to use the original name, though this
+ * is very likely to fail later if there are format
+ * specs in the string. Note that truncation cannot
+ * happen here as long as both buffers have the same
+ * size!
+ */
strlcpy(filename, filespec, sizeof(filename));
+ }
+
+ /*
+ * Check the file name for sanity. This might/will rule out file
+ * names that would be legal but problematic, and it blocks
+ * directory traversal.
+ */
+ if (!is_safe_filename(filename)) {
+ ctl_printf("saveconfig rejects unsafe file name '%s'",
+ filename);
+ ctl_flushpkt(0);
+ msyslog(LOG_NOTICE,
+ "saveconfig rejects unsafe file name from %s",
+ stoa(&rbufp->recv_srcadr));
+ return;
+ }
+
+ /*
+ * XXX: This next test may not be needed with is_safe_filename()
+ */
/* block directory/drive traversal */
/* TALOS-CAN-0062: block directory traversal for VMS, too */
@@ -968,38 +1097,49 @@ save_config(
ctl_putdata(reply, strlen(reply), 0);
ctl_flushpkt(0);
msyslog(LOG_NOTICE,
- "saveconfig with path from %s rejected",
+ "saveconfig rejects unsafe file name from %s",
stoa(&rbufp->recv_srcadr));
return;
}
- snprintf(fullpath, sizeof(fullpath), "%s%s",
- saveconfigdir, filename);
+ /* concatenation of directory and path can cause another
+ * truncation...
+ */
+ prc = snprintf(fullpath, sizeof(fullpath), "%s%s",
+ saveconfigdir, filename);
+ if (prc < 0 || prc >= sizeof(fullpath)) {
+ ctl_printf("saveconfig exceeded maximum path length (%u)",
+ (u_int)sizeof(fullpath));
+ ctl_flushpkt(0);
+ msyslog(LOG_NOTICE,
+ "saveconfig exceeded maximum path length from %s",
+ stoa(&rbufp->recv_srcadr));
+ return;
+ }
- fd = open(fullpath, O_CREAT | O_TRUNC | O_WRONLY,
- S_IRUSR | S_IWUSR);
+ fd = open(fullpath, openmode, S_IRUSR | S_IWUSR);
if (-1 == fd)
fptr = NULL;
else
fptr = fdopen(fd, "w");
if (NULL == fptr || -1 == dump_all_config_trees(fptr, 1)) {
- snprintf(reply, sizeof(reply),
- "Unable to save configuration to file %s",
- filename);
+ ctl_printf("Unable to save configuration to file '%s': %m",
+ filename);
msyslog(LOG_ERR,
"saveconfig %s from %s failed", filename,
stoa(&rbufp->recv_srcadr));
} else {
- snprintf(reply, sizeof(reply),
- "Configuration saved to %s", filename);
+ ctl_printf("Configuration saved to '%s'", filename);
msyslog(LOG_NOTICE,
- "Configuration saved to %s (requested by %s)",
+ "Configuration saved to '%s' (requested by %s)",
fullpath, stoa(&rbufp->recv_srcadr));
/*
* save the output filename in system variable
* savedconfig, retrieved with:
* ntpq -c "rv 0 savedconfig"
+ * Note: the way 'savedconfig' is defined makes overflow
+ * checks unnecessary here.
*/
snprintf(savedconfig, sizeof(savedconfig), "%s%s",
savedconfig_eq, filename);
@@ -1009,11 +1149,9 @@ save_config(
if (NULL != fptr)
fclose(fptr);
#else /* !SAVECONFIG follows */
- snprintf(reply, sizeof(reply),
- "saveconfig unavailable, configured with --disable-saveconfig");
-#endif
-
- ctl_putdata(reply, strlen(reply), 0);
+ ctl_printf("%s",
+ "saveconfig unavailable, configured with --disable-saveconfig");
+#endif
ctl_flushpkt(0);
}
@@ -1757,6 +1895,29 @@ ctl_putarray(
ctl_putdata(buffer, (unsigned)(cp - buffer), 0);
}
+/*
+ * ctl_printf - put a formatted string into the data buffer
+ */
+static void
+ctl_printf(
+ const char * fmt,
+ ...
+ )
+{
+ static const char * ellipsis = "[...]";
+ va_list va;
+ char fmtbuf[128];
+ int rc;
+
+ va_start(va, fmt);
+ rc = vsnprintf(fmtbuf, sizeof(fmtbuf), fmt, va);
+ va_end(va);
+ if (rc < 0 || rc >= sizeof(fmtbuf))
+ strcpy(fmtbuf + sizeof(fmtbuf) - strlen(ellipsis) - 1,
+ ellipsis);
+ ctl_putdata(fmtbuf, strlen(fmtbuf), 0);
+}
+
/*
* ctl_putsys - output a system variable
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-11 19:00:03 +0000