Gitweb:
https://git.exim.org/exim.git/commitdiff/163144aab02a47427340d0ecc75e2abde675f4c9
Commit: 163144aab02a47427340d0ecc75e2abde675f4c9
Parent: 1ea7f48754621db22ec40b6362823433d54bda62
Author: Jeremy Harris <jgh146exb@???>
AuthorDate: Sat Jan 11 18:07:10 2020 +0000
Committer: Jeremy Harris <jgh146exb@???>
CommitDate: Sat Jan 11 20:45:29 2020 +0000
$local_part_verified
---
doc/doc-docbook/spec.xfpt | 32 +++++++++++++++++++++++++++-----
doc/doc-txt/NewStuff | 3 +++
src/src/acl.c | 28 ++++++++++++----------------
src/src/configure.default | 2 +-
src/src/expand.c | 1 +
src/src/globals.c | 1 +
src/src/globals.h | 1 +
src/src/route.c | 4 +++-
src/src/routers/rf_get_errors_address.c | 8 ++++----
test/confs/0005 | 2 +-
10 files changed, 54 insertions(+), 28 deletions(-)
diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
index 241540c..254ed69 100644
--- a/doc/doc-docbook/spec.xfpt
+++ b/doc/doc-docbook/spec.xfpt
@@ -6362,7 +6362,7 @@ All other options are defaulted.
.code
local_delivery:
driver = appendfile
- file = /var/mail/$home
+ file = /var/mail/$local_part_verified
delivery_date_add
envelope_to_add
return_path_add
@@ -6370,7 +6370,17 @@ local_delivery:
# mode = 0660
.endd
This &(appendfile)& transport is used for local delivery to user mailboxes in
-traditional BSD mailbox format. By default it runs under the uid and gid of the
+traditional BSD mailbox format.
+
+.new
+We prefer to avoid using &$local_part$& directly to define the mailbox filename,
+as it is provided by a potential bad actor.
+Instead we use &$local_part_verified$&,
+the result of looking up &$local_part$& in the user database
+(done by using &%check_local_user%& in the the router).
+.wen
+
+By default &(appendfile)& runs under the uid and gid of the
local user, which requires the sticky bit to be set on the &_/var/mail_&
directory. Some systems use the alternative approach of running mail deliveries
under a particular group instead of using the sticky bit. The commented options
@@ -12202,6 +12212,7 @@ the complete argument of the ETRN command (see section &<<SECTETRN>>&).
.cindex "tainted data"
If the origin of the data is an incoming message,
the result of expanding this variable is tainted.
+See also &$domain_verified$&.
.wen
@@ -12402,15 +12413,18 @@ once.
If the origin of the data is an incoming message,
the result of expanding this variable is tainted.
-&*Warning*&: the content of this variable is usually provided by a potential attacker.
+&*Warning*&: the content of this variable is usually provided by a potential
+attacker.
Consider carefully the implications of using it unvalidated as a name
for file access.
This presents issues for users' &_.forward_& and filter files.
-For traditional full user accounts, use &%check_local_users%& and the &$home$&
-variable rather than this one.
+For traditional full user accounts, use &%check_local_users%& and the
+&$local_part_verified$& variable rather than this one.
For virtual users, store a suitable pathname component in the database
which is used for account name validation, and use that retrieved value
rather than this variable.
+If needed, use a router &%address_data%& or &%set%& option for
+the retrieved data.
.wen
.vindex "&$local_part_prefix$&"
@@ -12479,6 +12493,14 @@ When an address is being routed or delivered, and a
specific suffix for the local part was recognized, it is available in this
variable, having been removed from &$local_part$&.
+.new
+.vitem &$local_part_verified$&
+.vindex "&$local_part_verified$&"
+If the router generic option &%check_local_part%& has run successfully,
+this variable has the user database version of &$local_part$&.
+Such values are not tainted and hence usable for building file names.
+.wen
+
.vitem &$local_scan_data$&
.vindex "&$local_scan_data$&"
This variable contains the text returned by the &[local_scan()]& function when
diff --git a/doc/doc-txt/NewStuff b/doc/doc-txt/NewStuff
index 6b163b8..8a00bfc 100644
--- a/doc/doc-txt/NewStuff
+++ b/doc/doc-txt/NewStuff
@@ -21,6 +21,9 @@ Version 4.94
driver for PLAIN; only against itself for SCRAM-SHA-1 and SCRAM-SHA-1-PLUS
methods.
+ 5. Variable $local_part_verified, set by the router check_local_part condition
+ with untainted data.
+
Version 4.93
------------
diff --git a/src/src/acl.c b/src/src/acl.c
index 799af39..7284831 100644
--- a/src/src/acl.c
+++ b/src/src/acl.c
@@ -866,11 +866,10 @@ while ((s = (*func)()) != NULL)
{
uschar *endptr;
- if (Ustrncmp(s, "acl_c", 5) != 0 &&
- Ustrncmp(s, "acl_m", 5) != 0)
+ if (Ustrncmp(s, "acl_c", 5) != 0 && Ustrncmp(s, "acl_m", 5) != 0)
{
*error = string_sprintf("invalid variable name after \"set\" in ACL "
- "modifier \"set %s\" (must start \"acl_c\" or \"acl_m\")", s);
+ "modifier \"set %s\" (must start \"acl_c\" or \"acl_m\")", s);
return NULL;
}
@@ -878,19 +877,19 @@ while ((s = (*func)()) != NULL)
if (!isdigit(*endptr) && *endptr != '_')
{
*error = string_sprintf("invalid variable name after \"set\" in ACL "
- "modifier \"set %s\" (digit or underscore must follow acl_c or acl_m)",
- s);
+ "modifier \"set %s\" (digit or underscore must follow acl_c or acl_m)",
+ s);
return NULL;
}
- while (*endptr != 0 && *endptr != '=' && !isspace(*endptr))
+ while (*endptr && *endptr != '=' && !isspace(*endptr))
{
if (!isalnum(*endptr) && *endptr != '_')
- {
- *error = string_sprintf("invalid character \"%c\" in variable name "
- "in ACL modifier \"set %s\"", *endptr, s);
- return NULL;
- }
+ {
+ *error = string_sprintf("invalid character \"%c\" in variable name "
+ "in ACL modifier \"set %s\"", *endptr, s);
+ return NULL;
+ }
endptr++;
}
@@ -3634,15 +3633,12 @@ for (; cb; cb = cb->next)
sender_address_cache, -1, 0, CUSS &sender_data);
break;
- /* Connection variables must persist forever */
+ /* Connection variables must persist forever; message variables not */
case ACLC_SET:
{
int old_pool = store_pool;
- if ( cb->u.varname[0] == 'c'
-#ifndef DISABLE_DKIM
- || cb->u.varname[0] == 'd'
-#endif
+ if ( cb->u.varname[0] != 'm'
#ifndef DISABLE_EVENT
|| event_name /* An event is being delivered */
#endif
diff --git a/src/src/configure.default b/src/src/configure.default
index 08f5a9d..93aa167 100644
--- a/src/src/configure.default
+++ b/src/src/configure.default
@@ -863,7 +863,7 @@ smarthost_smtp:
local_delivery:
driver = appendfile
- file = /var/mail/$home
+ file = /var/mail/$local_part_verified
delivery_date_add
envelope_to_add
return_path_add
diff --git a/src/src/expand.c b/src/src/expand.c
index 366cd73..be6cd61 100644
--- a/src/src/expand.c
+++ b/src/src/expand.c
@@ -587,6 +587,7 @@ static var_entry var_table[] = {
{ "local_part_data", vtype_stringptr, &deliver_localpart_data },
{ "local_part_prefix", vtype_stringptr, &deliver_localpart_prefix },
{ "local_part_suffix", vtype_stringptr, &deliver_localpart_suffix },
+ { "local_part_verified", vtype_stringptr, &deliver_localpart_verified },
#ifdef HAVE_LOCAL_SCAN
{ "local_scan_data", vtype_stringptr, &local_scan_data },
#endif
diff --git a/src/src/globals.c b/src/src/globals.c
index 15ad4e9..edd1edb 100644
--- a/src/src/globals.c
+++ b/src/src/globals.c
@@ -817,6 +817,7 @@ uschar *deliver_localpart_orig = NULL;
uschar *deliver_localpart_parent = NULL;
uschar *deliver_localpart_prefix = NULL;
uschar *deliver_localpart_suffix = NULL;
+uschar *deliver_localpart_verified = NULL;
uschar *deliver_out_buffer = NULL;
int deliver_queue_load_max = -1;
address_item *deliver_recipients = NULL;
diff --git a/src/src/globals.h b/src/src/globals.h
index bb66cb2..9dfa4a7 100644
--- a/src/src/globals.h
+++ b/src/src/globals.h
@@ -488,6 +488,7 @@ extern uschar *deliver_localpart_orig; /* The original local part for delivery *
extern uschar *deliver_localpart_parent; /* The parent local part for delivery */
extern uschar *deliver_localpart_prefix; /* The stripped prefix, if any */
extern uschar *deliver_localpart_suffix; /* The stripped suffix, if any */
+extern uschar *deliver_localpart_verified; /* de-tainted by check_local_part */
extern uschar *deliver_out_buffer; /* Buffer for copying file */
extern int deliver_queue_load_max; /* Different value for queue running */
extern address_item *deliver_recipients; /* Current set of addresses */
diff --git a/src/src/route.c b/src/src/route.c
index c6119ee..16ce72c 100644
--- a/src/src/route.c
+++ b/src/src/route.c
@@ -927,7 +927,7 @@ if ((rc = route_check_dls(r->name, US"local_parts", r->local_parts,
login of a local user. Note: the third argument to route_finduser() must be
NULL here, to prevent a numeric string being taken as a numeric uid. If the
user is found, set deliver_home to the home directory, and also set
-local_user_{uid,gid}. */
+local_user_{uid,gid} and local_part_verified. */
if (r->check_local_user)
{
@@ -938,6 +938,7 @@ if (r->check_local_user)
r->name, addr->local_part);
return SKIP;
}
+ deliver_localpart_verified = string_copy(US (*pw)->pw_name);
deliver_home = string_copy(US (*pw)->pw_dir);
local_user_gid = (*pw)->pw_gid;
local_user_uid = (*pw)->pw_uid;
@@ -1670,6 +1671,7 @@ for (r = addr->start_router ? addr->start_router : routers; r; r = nextr)
the local part sorted. */
router_name = r->name;
+ deliver_localpart_verified = NULL;
deliver_set_expansions(addr);
/* For convenience, the pre-router checks are in a separate function, which
diff --git a/src/src/routers/rf_get_errors_address.c b/src/src/routers/rf_get_errors_address.c
index 858c806..9ef4680 100644
--- a/src/src/routers/rf_get_errors_address.c
+++ b/src/src/routers/rf_get_errors_address.c
@@ -88,13 +88,13 @@ else
const uschar *address_expansions_save[ADDRESS_EXPANSIONS_COUNT];
address_item *snew = deliver_make_addr(s, FALSE);
- if (sender_address != NULL)
+ if (sender_address)
{
save1 = sender_address[0];
sender_address[0] = 0;
}
- for (i = 0, p = address_expansions; *p != NULL;)
+ for (i = 0, p = address_expansions; *p;)
address_expansions_save[i++] = **p++;
f.address_test_mode = FALSE;
@@ -119,10 +119,10 @@ else
debug_printf("------ End verifying errors address %s ------\n", s);
f.address_test_mode = save_address_test_mode;
- for (i = 0, p = address_expansions; *p != NULL;)
+ for (i = 0, p = address_expansions; *p; )
**p++ = address_expansions_save[i++];
- if (sender_address != NULL) sender_address[0] = save1;
+ if (sender_address) sender_address[0] = save1;
}
return OK;
diff --git a/test/confs/0005 b/test/confs/0005
index 7c0689c..77b7910 100644
--- a/test/confs/0005
+++ b/test/confs/0005
@@ -52,7 +52,7 @@ local_delivery:
driver = appendfile
delivery_date_add
envelope_to_add
- file = DIR/test-mail/$local_part
+ file = DIR/test-mail/$local_part_verified
headers_add = "X-body-linecount: $body_linecount\n\
X-message-linecount: $message_linecount\n\
X-received-count: $received_count"
