On Oct 21, 2014, at 9:27 AM, Geert Janssens <geert.gnuc...@kobaltwit.be> wrote:
> 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. Wow. That’s an incredible failure for something that’s promoted as an application scripting language. > > 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… That might be overstating it a bit, though it would require a fair amount of surgery to convert all of the existing reports. That might be doable with a bit of perl to extract the data into an ini-style file and remove the block from each report’s scheme file. Not that I like the current system: I’d prefer a report definition consisting of a query for the data feeding canned graphing and table-drawing functions with CSS to make it pretty. Depending on how the query part is implemented it might run the risk of SQL injection, but that’s less of a risk than allowing arbitrary code as we do now. I would prefer that such a thing be written in a different language from Scheme. Regards, John Ralls _______________________________________________ gnucash-devel mailing list gnucash-devel@gnucash.org https://lists.gnucash.org/mailman/listinfo/gnucash-devel
