The `risky-local-variable' blacklist

[Davis Herring]

(Apologies in advance for a long message, but this is a long issue.)

While looking at diffs for `timeclock.el', I noticed the addition of a
risk-local-variable declaration for "timeclock-mode-string".  This is
certainly justified, but calls forth a bigger concern: is it wise to apply
a 'trust by default' policy when such innocuous-looking variables as that
mode-string can completely compromise a user's security (including
modifying configurations for further attacks)?  Not to mention the threat
posed by Lisp authors (who possibly write popular packages not picked over
by the maintainers) that accidentally endanger their users (unless and
until the author (if needed) learns about risky variables, recognizes the
risk, and changes the code, and the users upgrade)...

The danger is especially pertinent since `enable-local-variables' defaults 
to t and `normal-mode' overrides any paranoid setting of that variable 
when called interactively or without its (optional!) argument.

It seems much safer in the long term to allow only specific variables to
be set by a local-variables specification, especially since relatively few
variables are useful to set that way.  Of course, more things should be
possible with prompting, but such prompting should certainly (or at least
by default) be with `yes-or-no-p'.

Fortunately, this does not break existing code in notable ways, since the
`risky-local-variable' property would then be a no-op.  At worst, the user
encounters prompts for files which did not previously cause them.  This 
may be a bit confusing, but if the files really are legitimate, that will 
be evident at the time of the prompt.

Also, the protection versus honoring `eval' settings when root does little 
good since it does not apply in other cases; anyone interested in rooting 
via Emacs surely knows this.

Here is a list of local-variable settings used by the 21.3.1 lisp files:
calendar/calendar.el         byte-compile-dynamic    t
gnus/gnus-cite.el            (coding)                iso-latin-1
gnus/gnus-spec.el            (coding)                iso-latin-1
gnus/message.el              (coding)                iso-latin-1
gnus/nnultimate.el           (coding)                iso-latin-1
international/titdic-cnv.el  (coding)                (iso-2022-7bit)
play/landmark.el             outline-layout          (0 : -1 -1 0)
progmodes/ebrowse.el         (eval)                  (useless)
tildify.el                   (coding)                iso-latin-2
version.el                   version-control         never
allout.el                    outline-layout          (0 : -1 -1 0)
dired-x.el                   byte-compile-dynamic    t
icomplete.el                 outline-layout          (-2 :)
wid-edit.el                  byte-compile-dynamic    t

(titdic-cnv.el is adding that coding specification into a buffer.)

We see a few codings (which are safe), a few `outline-layout's (safe), a 
few `byte-compile-dynamic's (safe), a `version-control' (mostly safe; can 
risk minor data loss), and one (eval) that does nothing but `put' safe 
values on `lisp-indent-hook' properties.  From this sample, there does not 
appear to be much variety in (legitimate) local-variable settings; surely 
anything truly useful in a file will be defined either globally or by the 
mode for the file, and thus known and established by the time it is set.

Therefore, suggestions (all of them with some urgency):

* Drop `risky-local-variable' property: everything is risky unless 
blessed as safe.

* Do not make `compile-command' safe; the default value for M-x compile is
of absolutely no use if one must first make sure that it has not in fact
become "rm -rf ~/*" since you started Emacs.  Of course, the risk of 
double-tapping RET after "M-x compile" is notable as well.

* Do not provide options for trusting non-"safe" variables (much less
eval) without prompting -- at the very least, force prompting for root!

* Prompt, when prompting is necessary, with `yes-or-no-p' unless the user 
is not root and has asked for `y-or-n-p'.

* Provide a simple option to entirely prevent the setting or evaluation of
anything (except, say, mode and coding system) by a file, for use by the
paranoid.  Make this be the default for root.

* In conjunction with the option for wholesale supression of this feature, 
do not make `normal-mode' ignore these protections -- although with only
whitelisting and prompting, there is less for it to ignore.

* Fix the documentation of `enable-local-eval' so that it does not say 
that `normal-mode' acts as if it were t (fortunately it does not).

* Rename (or at least re-document) `ignored-local-variables': it does not
ignore anything but merely causes "riskiness".

* Refine the check in `hack-one-local-variable' for harmless properties: 
for instance, it appears to allow setting lisp-indent-hook to be a macro, 
or to be bytecode.  It would probably be better to have a safe-property 
property for safe property symbols, handled as is safe-local-variable.

* Do not use `enable-local-eval' as a local flag to prevent dangerous bugs 
in its handling; have `hack-one-local-variable' accept a flag FORCE which 
overrides all protections, and have it return t if FORCE was set or if the 
user accepted the risk when prompted.  (This might be better done by 
factoring the risk-checking and prompting out of 
`hack-one-local-variable'.)

* (The doozy, of course:) Mark all safe local variables as such.

I would be happy to work on all these projects, but desire opinions (on
the whole issue and on the suggested changes) and suggestions before 
undertaking such an endeavor.

Davis Herring

-- 
This product is sold by volume, not by mass.  If it seems too dense or too 
sparse, it means mass-energy conversion has occurred during shipping.