Security implications of loading custom reports (was: Re: Custom reports do not load after upgrade to 2.6.4 on OSX)

[Geert Janssens]

I have moved this thread from gnucash-user as it was getting rather technical...

On Tuesday 21 October 2014 08:43:04 John Ralls wrote:
> > > 
> > > One possible solution is to get Guile out of the file-opening
> > > loop.
> > > Having a configuration file that’s directly executed by Guile is a
> > > rather glaring security hole; it would be much better to make it
> > > an
> > > ini-style file and do everything in C/C++ up to passing the
> > > contents
> > > of the files to a Guile function that registers them as add-on
> > > reports. That’s not really a suitable change for the maint branch,
> > > but we should consider it for 2.8.
> > 
> > I understand your reasoning. However a custom report in itself is
> > essentially unrestricted scheme code as well. So how is loading
> > such a file via C/C++ any less of a glaring security hole than
> > having it done in guile ?
> It’s not if the C/C++ code just passes arbitrary code to Guile for
> immediate execution. I’m proposing that the custom report should be
> passed to the report-registration code instead so that the report
> must be selected from the menu to execute.

I'm not sure this is possible in guile only. A report is written as a guile module. Loading the 
module already executes code (gnc:define-report). That code can be abused do bad things 
when loading a custom report.

Thinking further on your .ini file we could perhaps indeed split the current report in two parts:
- an ini file as you propose which holds the parameters we currently set in the call to gnc:define-
report
- a guile module with all the functions that can be called by the report.
That would reduce the execution of arbitrary code to when the report is effectively called.

Note that our built-in reports use the same loading mechanism and for user-friendliness we 
probably want to keep only one load interface. Which means we would want to convert our 
built-in reports as well.

Next if the goal is to prevent arbitrary code execution we will also have to rewrite the saved-
report code, because it is also running a guile file from DOT_GNUCASH_DIR. And loading of 
stylesheets.

By now it starts to look like a rewrite of the report system. Which I'm not sure I would want to 
do in guile again...

> It would be even better if
> reports could be sandboxed, but that’s probably too hard.
> 
Agreed.

Geert