diff options
author | Kaz Kylheku <kaz@kylheku.com> | 2020-04-09 06:18:46 -0700 |
---|---|---|
committer | Kaz Kylheku <kaz@kylheku.com> | 2020-04-09 06:18:46 -0700 |
commit | 02b83600e20bbf7c61e09c9ad40ff35433da0c74 (patch) | |
tree | 4f98524019c22c1855e9622cf304524caceff407 /parser.c | |
parent | dd23b426d1a5c1790a4886201628476bf73fe7b8 (diff) | |
download | txr-02b83600e20bbf7c61e09c9ad40ff35433da0c74.tar.gz txr-02b83600e20bbf7c61e09c9ad40ff35433da0c74.tar.bz2 txr-02b83600e20bbf7c61e09c9ad40ff35433da0c74.zip |
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.
Diffstat (limited to 'parser.c')
-rw-r--r-- | parser.c | 32 |
1 files changed, 27 insertions, 5 deletions
@@ -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); |