emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: SES local variables to define printers


From: Vincent Belaïche
Subject: Re: SES local variables to define printers
Date: Fri, 24 May 2013 07:46:12 +0200

Answers embedded below:

> From: address@hidden
> To: address@hidden
> CC: address@hidden
> Subject: Re: SES local variables to define printers
> Date: Thu, 23 May 2013 21:06:45 -0400
> 
> 
> I haven't quite followed what you are trying to do (I don't know what a
> SES "printer" is), 

"SES Printers" are functions which take some cell value (like an integer
or a string, or whatever lisp object may be stored in a cell) at input
and return a string at output, that string is then align, fill or
truncated and displayed in a cell.

Customizing SES printer allows to do some fancy display for some cells
(e.g. display *like this* when the value is out of some bound).

> but as a general comment: please, please don't (re)invent your own
> version of local variable handling unless you are really, really sure
> you need special handling that is not provided by normal file or
> directory local variables (or local eval). 

I don't want to reinvent the wheel, this is exactly why I am not
satisfied with the hack which I did, and I am seeking guidance on this
forum. 

> (And be really careful if you do implement it, since it can be a
> security hole.)
> 

I am well aware of that, I won't commit anything before being sure that
it is in-line with EMACS general security policy.

> 
> Vincent Belaïche wrote:
> 
> > That sort of trick was to define a file local variable foo to a lambda
> > expression defining a printer function, and then use the foo symbol as a
> > printer when setting the printer for a cell.
> [...]
> > By the way, that kind of things was a security breach because you allow
> > to call a function defined in the file without any control.
> 
> !!!
> 

Yes, I fully agree with the "!!!", but this is what was there with emacs
23.xx or so. When I used that possibility the first time I did not
really realize that this could be a hold for malicious spreadsheets.


> > Now, I would like to have again the same sort of feature in SES,
> 
> !!!
> 

Yes, I would like the same possibility to define file local printer
function, but with less security hazards and more user control.

> > so I did a quick hack herein attached 
> 
> !!!
> 
> "Security breach" + "quick hack" = fun times
> 

I needed the feature really urgently, so I did that only for myself, I
do not recommend the patch attached for incorporating that into official
EMACS.


> > - is that possible to check that when a function is executing, that
> > takes a reasonable time, and if not to interact with the use and ask
> > whether he/she would like to double that time
> 
> Your printer function `(lambda (arg) (shell-command "rm -rf /"))'
> has been running for 30 seconds. Run for another 60?
> 
> Doesn't help much...
> 

Typically you would have checked that the printer function does not do
any potential border effect in the first place, so a function calling
`shell-command' would have been screened for that before being
called. The case which would like to prevent if function with infinite
loops that simply would get the user stuck, but without doing much harm
--- well they could also clutter the heap, so maybe that should also be
checked that the function does not take too much memory.

> > +(defcustom ses-enable-local-variables nil
> > + "Non-nil if SES should process local-variables lists in ses buffers.
> 
> Why is this needed - what's wrong with the normal enable-local-variables?
> Why should there be a special variable that controls local variables
> only in SES files?
> 

With this additional variable you may allow to have file local variables
for some type of variables like calc-float-format or calc-number-radix
that influence that way some formatting function process their input,
but at the same time prevent any file local printer functions.

> > +\(You can explicitly request processing the local-variables by
> > +executing `(hack-local-variables)'). Local variables are useful
> > +to define file local printers or values but raise a security
> > +issue if the printer function is used to do border effects. If
> > +you select `Filename test', then you should configure a function
> > +symbol or lambda expression which takes one argument, then the
> > +local variables are processed iff the buffer file name passed to
> 
> Don't use "iff" in doc strings.
> 

Ok, that is not English, neither I am ;-)...

> > +this function returns a non nil. For instance you could configure:
> 
> "returns non-nil".
> 
> > + (lambda (x)
> > + (string-match \"^/dir/where/local/var/are/allowed\" 
> > + (expand-file-name x)))
> 
> Sounds like dir-locals. Why not just use a dir-locals file?
> 

Simply because I did not know about dir-locals. You mean info node
`(emacs) Directory Variables'. 

Well, I agree that this would help in the case for which I needed that
hack. But in essence, whether the variable is defined locally to a
directory or for a single file does not per se solve all security
issues: imagine that you download from the Internet a whole tar-zipped
directory tree and you have some malicious printer functions defined
dir-locally in this tree, then you would still have the issue. With the
defcustom you check that 

> > +"
> > + :type '(choice
> > + (const :tag "No" nil)
> > + (const :tag "Yes" t)
> > + (function :tag "Filename test"))
> > + :group 'ses)
> 
> You would need to add
> 
> :risky t
> 

Yes, you are right. Anyway the first thing we should solve is whether
there is not a better way to do that, and how to do some checks on that
printer function to guess whether it can be malicious or not.

> > (functionp printer)
> > + (and (symbolp printer) (boundp printer) (functionp (symbol-value 
> > printer)))
> 
> What is this for?
> 


In that case printer is not a function but a symbol pointing at a file
local variable, and the "value" slot of this symbol is what contains the
printer function.

> > + (and (symbolp printer)
> > + (boundp printer)
> > + (functionp (symbol-value printer))
> > + (setq printer (symbol-value printer)))
> 
> ? Likewise.
> 

Same answer.

> > (setq value (funcall printer (or value "")))
> > (if (stringp value)
> > value
> > @@ -1899,9 +1925,17 @@
> > (unless (and (boundp 'ses--deferred-narrow)
> > (eq ses--deferred-narrow 'ses-mode))
> > (kill-all-local-variables)
> > + (setq major-mode 'ses-mode)
> > + (and
> > + enable-local-variables
> > + ses-enable-local-variables
> > + (or (eq ses-enable-local-variables t)
> > + (let ((bfn (buffer-file-name)))
> > + (and (stringp bfn)
> > + (funcall ses-enable-local-variables bfn))))
> 
> Ironically, ses-enable-local-variables is itself a potential security hole...

Yes, you are right I have to tag this variable as not to be set
file-locally, I missed that one, thank you for suggesting the `:risky
t'.

   Vincent.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]