From 97bbe68363ccf2de0c07f67170ec64a8b4d62592 Mon Sep 17 00:00:00 2001
From: TJ Saunders <tj@castaglia.org>
Date: Sun, 6 Aug 2023 13:16:26 -0700
Subject: [PATCH] Issue #1683: Avoid an edge case when handling unexpectedly
formatted input text from client, caused by quote/backslash semantics, by
skipping those semantics.
Backported to 1.3.6e by Paul Howarth <paul@city-fan.org>
diff -up a/include/str.h.cve-2023-51713 b/include/str.h
--- a/include/str.h
+++ b/include/str.h
@@ -1,6 +1,6 @@
/*
* ProFTPD - FTP server daemon
- * Copyright (c) 2008-2017 The ProFTPD Project team
+ * Copyright (c) 2008-2023 The ProFTPD Project team
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -121,6 +121,7 @@ const char *pr_gid2str(pool *, gid_t);
#define PR_STR_FL_PRESERVE_COMMENTS 0x0001
#define PR_STR_FL_PRESERVE_WHITESPACE 0x0002
#define PR_STR_FL_IGNORE_CASE 0x0004
+#define PR_STR_FL_IGNORE_QUOTES 0x0008
char *pr_str_get_token(char **, char *);
char *pr_str_get_token2(char **, char *, size_t *);
diff -up a/src/main.c.cve-2023-51713 b/src/main.c
--- a/src/main.c
+++ b/src/main.c
@@ -795,8 +795,24 @@ static cmd_rec *make_ftp_cmd(pool *p, char *buf, size_t buflen, int flags) {
return NULL;
}
+ /* By default, pr_str_get_word will handle quotes and backslashes for
+ * escaping characters. This can produce words which are shorter, use
+ * fewer bytes than the corresponding input buffer.
+ *
+ * In this particular situation, we use the length of this initial word
+ * for determining the length of the remaining buffer bytes, assumed to
+ * contain the FTP command arguments. If this initial word is thus
+ * unexpectedly "shorter", due to nonconformant FTP text, it can lead
+ * the subsequent buffer scan, looking for CRNUL sequencees, to access
+ * unexpected memory addresses (Issue #1683).
+ *
+ * Thus for this particular situation, we tell the function to ignore/skip
+ * such quote/backslash semantics, and treat them as any other character
+ * using the IGNORE_QUOTES flag.
+ */
+
ptr = buf;
- wrd = pr_str_get_word(&ptr, str_flags);
+ wrd = pr_str_get_word(&ptr, str_flags|PR_STR_FL_IGNORE_QUOTES);
if (wrd == NULL) {
/* Nothing there...bail out. */
pr_trace_msg("ctrl", 5, "command '%s' is empty, ignoring", buf);
@@ -804,6 +820,11 @@ static cmd_rec *make_ftp_cmd(pool *p, char *buf, size_t buflen, int flags) {
return NULL;
}
+ /* Note that this first word is the FTP command. This is why we make
+ * use of the ptr buffer, which advances through the input buffer as
+ * we read words from the buffer.
+ */
+
subpool = make_sub_pool(p);
pr_pool_tag(subpool, "make_ftp_cmd pool");
cmd = pcalloc(subpool, sizeof(cmd_rec));
@@ -830,6 +851,7 @@ static cmd_rec *make_ftp_cmd(pool *p, char *buf, size_t buflen, int flags) {
arg_len = buflen - strlen(wrd);
arg = pcalloc(cmd->pool, arg_len + 1);
+ /* Remember that ptr here is advanced past the first word. */
for (i = 0, j = 0; i < arg_len; i++) {
pr_signals_handle();
if (i > 1 &&
@@ -844,9 +866,7 @@ static cmd_rec *make_ftp_cmd(pool *p, ch
}
}
- cmd->arg = arg;
-
- if (have_crnul) {
+ if (have_crnul == TRUE) {
char *dup_arg;
/* Now make a copy of the stripped argument; this is what we need to
@@ -856,6 +876,11 @@ static cmd_rec *make_ftp_cmd(pool *p, ch
ptr = dup_arg;
}
+ cmd->arg = arg;
+
+ /* Now we can read the remamining words, as command arguments, from the
+ * input buffer.
+ */
while ((wrd = pr_str_get_word(&ptr, str_flags)) != NULL) {
pr_signals_handle();
*((char **) push_array(tarr)) = pstrdup(cmd->pool, wrd);
diff -up a/src/str.c.cve-2023-51713 b/src/str.c
--- a/src/str.c
+++ b/src/str.c
@@ -1,6 +1,6 @@
/*
* ProFTPD - FTP server daemon
- * Copyright (c) 2008-2017 The ProFTPD Project team
+ * Copyright (c) 2008-2023 The ProFTPD Project team
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -1209,7 +1209,7 @@ int pr_str_get_nbytes(const char *str, const char *units, off_t *nbytes) {
char *pr_str_get_word(char **cp, int flags) {
char *res, *dst;
- char quote_mode = 0;
+ int quote_mode = FALSE;
if (cp == NULL ||
!*cp ||
@@ -1238,24 +1238,28 @@ char *pr_str_get_word(char **cp, int flags) {
}
}
- if (**cp == '\"') {
- quote_mode++;
- (*cp)++;
+ if (!(flags & PR_STR_FL_IGNORE_QUOTES)) {
+ if (**cp == '\"') {
+ quote_mode = TRUE;
+ (*cp)++;
+ }
}
while (**cp && (quote_mode ? (**cp != '\"') : !PR_ISSPACE(**cp))) {
pr_signals_handle();
- if (**cp == '\\' && quote_mode) {
-
+ if (**cp == '\\' &&
+ quote_mode == TRUE) {
/* Escaped char */
if (*((*cp)+1)) {
- *dst = *(++(*cp));
+ *dst++ = *(++(*cp));
+ (*cp)++;
+ continue;
}
}
*dst++ = **cp;
- ++(*cp);
+ (*cp)++;
}
if (**cp) {
diff -up a/tests/api/str.c.cve-2023-51713 b/tests/api/str.c
--- a/tests/api/str.c
+++ b/tests/api/str.c
@@ -1,6 +1,6 @@
/*
* ProFTPD - FTP server testsuite
- * Copyright (c) 2008-2017 The ProFTPD Project team
+ * Copyright (c) 2008-2023 The ProFTPD Project team
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -695,19 +695,23 @@ END_TEST
START_TEST (get_word_test) {
char *ok, *res, *str;
+ mark_point();
res = pr_str_get_word(NULL, 0);
fail_unless(res == NULL, "Failed to handle null arguments");
fail_unless(errno == EINVAL, "Failed to set errno to EINVAL");
+ mark_point();
str = NULL;
res = pr_str_get_word(&str, 0);
fail_unless(res == NULL, "Failed to handle null str argument");
fail_unless(errno == EINVAL, "Failed to set errno to EINVAL");
+ mark_point();
str = pstrdup(p, " ");
res = pr_str_get_word(&str, 0);
fail_unless(res == NULL, "Failed to handle whitespace argument");
+ mark_point();
str = pstrdup(p, " foo");
res = pr_str_get_word(&str, PR_STR_FL_PRESERVE_WHITESPACE);
fail_unless(res != NULL, "Failed to handle whitespace argument: %s",
@@ -723,6 +727,7 @@ START_TEST (get_word_test) {
ok = "foo";
fail_unless(strcmp(res, ok) == 0, "Expected '%s', got '%s'", ok, res);
+ mark_point();
str = pstrdup(p, " # foo");
res = pr_str_get_word(&str, 0);
fail_unless(res == NULL, "Failed to handle commented argument");
@@ -742,6 +747,8 @@ START_TEST (get_word_test) {
fail_unless(strcmp(res, ok) == 0, "Expected '%s', got '%s'", ok, res);
/* Test multiple embedded quotes. */
+
+ mark_point();
str = pstrdup(p, "foo \"bar baz\" qux \"quz norf\"");
res = pr_str_get_word(&str, 0);
fail_unless(res != NULL, "Failed to handle quoted argument: %s",
@@ -770,6 +777,46 @@ START_TEST (get_word_test) {
ok = "quz norf";
fail_unless(strcmp(res, ok) == 0, "Expected '%s', got '%s'", ok, res);
+
+ /* Test embedded quotes with backslashes (Issue #1683). */
+ mark_point();
+
+ str = pstrdup(p, "\"\\\\SYST\"");
+ res = pr_str_get_word(&str, 0);
+ fail_unless(res != NULL, "Failed to handle quoted argument: %s",
+ strerror(errno));
+
+ ok = "\\SYST";
+ fail_unless(strcmp(res, ok) == 0, "Expected '%s', got '%s'", ok, res);
+
+ mark_point();
+ str = pstrdup(p, "\"\"\\\\SYST");
+ res = pr_str_get_word(&str, 0);
+ fail_unless(res != NULL, "Failed to handle quoted argument: %s",
+ strerror(errno));
+
+ /* Note that pr_str_get_word() is intended to be called multiple times
+ * on an advancing buffer, effectively tokenizing the buffer. This is
+ * why the function does NOT decrement its quote mode.
+ */
+ ok = "";
+ fail_unless(strcmp(res, ok) == 0, "Expected '%s', got '%s'", ok, res);
+
+ /* Now do the same tests with the IGNORE_QUOTES flag */
+ mark_point();
+
+ str = ok = pstrdup(p, "\"\\\\SYST\"");
+ res = pr_str_get_word(&str, PR_STR_FL_IGNORE_QUOTES);
+ fail_unless(res != NULL, "Failed to handle quoted argument: %s",
+ strerror(errno));
+ fail_unless(strcmp(res, ok) == 0, "Expected '%s', got '%s'", ok, res);
+
+ mark_point();
+ str = ok = pstrdup(p, "\"\"\\\\SYST");
+ res = pr_str_get_word(&str, PR_STR_FL_IGNORE_QUOTES);
+ fail_unless(res != NULL, "Failed to handle quoted argument: %s",
+ strerror(errno));
+ fail_unless(strcmp(res, ok) == 0, "Expected '%s', got '%s'", ok, res);
}
END_TEST