From 02b83600e20bbf7c61e09c9ad40ff35433da0c74 Mon Sep 17 00:00:00 2001
From: Kaz Kylheku <kaz@kylheku.com>
Date: Thu, 9 Apr 2020 06:18:46 -0700
Subject: repl: improve dotfile security tests.

We test the .txr_history file for bad permissions also, not
only .txr_profile. Though commands are not automatically
executed out of .txr_history, a user could execute a harmful
command due to not noticing the malicious modification.

An additional useful diagnostic is added: if a dotfile is
found to have the wrong permission, it's possible that this is
due to a poor umask setting. We check for a weak umask and
warn the user.

Note: the .txr_history check doesn't use the open stream,
therefore it is vulnerable to TOCTTOU race condition:
the file looks good, but between the time we verify this
and open the file to load it, the file has been replaced
by a malicious one.

* parser.c (report_security_problem): New static function,
factored out of load_rcfile. Includes umask test.
(load_rcfile): Call report_security_problem if the
.txr_profile is writable to others. Also, no need to call stat
any more; the path testing function now takes a stream
argument.
(repl): Check .txr_history for inappropriate writepermissions
also and call report_security_problem if so.

* sysif.c (umask_wrap): Change static function to external
linkage.

* sysif.c (umask_wrap): Declaration updated.
---
 parser.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

(limited to 'parser.c')

diff --git a/parser.c b/parser.c
index 7083353b..bbc1a427 100644
--- a/parser.c
+++ b/parser.c
@@ -824,6 +824,26 @@ val txr_parse(val source_in, val error_stream,
 
 #if HAVE_TERMIOS
 
+static void report_security_problem(val name)
+{
+  static int umask_warned;
+
+  format(std_output,
+         lit("** possible security problem: ~a is writable to others\n"),
+         name, nao);
+#if HAVE_SYS_STAT
+  if (!umask_warned++)
+  {
+    val um = umask_wrap(colon_k);
+    if ((c_num(um) & 022) != 022) {
+      format(std_output,
+             lit("** possible reason: your umask has an insecure value: ~,03o\n"),
+             um, nao);
+    }
+  }
+#endif
+}
+
 static void load_rcfile(val name)
 {
   val resolved_name;
@@ -837,10 +857,8 @@ static void load_rcfile(val name)
   open_txr_file(name, &lisp_p, &resolved_name, &stream);
 
   if (stream) {
-    if (!funcall1(path_private_to_me_p, stat_wrap(stream))) {
-      format(std_output,
-             lit("** possible security problem: ~a is writable to others\n"),
-             name, nao);
+    if (!funcall1(path_private_to_me_p, stream)) {
+      report_security_problem(name);
     } else {
       val saved_dyn_env = set_dyn_env(make_env(nil, nil, dyn_env));
       env_vbind(dyn_env, load_path_s, resolved_name);
@@ -1409,6 +1427,7 @@ val repl(val bindings, val in_stream, val out_stream, val env)
   val counter_sym = intern(lit("*n"), user_package);
   val var_counter_sym = intern(lit("*v"), user_package);
   val result_hash_sym = intern(lit("*r"), user_package);
+  val path_private_to_me_p =  intern(lit("path-private-to-me-p"), user_package);
   val result_hash = make_hash(nil, nil, nil);
   val done = nil;
   val counter = one;
@@ -1445,8 +1464,11 @@ val repl(val bindings, val in_stream, val out_stream, val env)
 
   lino_hist_set_max_len(ls, c_num(cdr(hist_len_var)));
 
-  if (histfile_w)
+  if (histfile_w) {
+    if (!funcall1(path_private_to_me_p, histfile))
+      report_security_problem(histfile);
     lino_hist_load(ls, histfile_w);
+  }
 
   lino_set_noninteractive(ls, opt_noninteractive);
 
-- 
cgit v1.2.3