[Lilypond-auto] [LilyIssues-auto] [testlilyissues:issues] #5243 Fix secu

Ok, I am attaching a prospective diff here. Why not make it a full review? I am not sure that this is the way to go ahead. This is as to yet untested: more likely than not it does not compile/run/work. It's more of a discussion basis than anything else.

The advantage of my approach is that it's API compatible and merely relies on an additional shell-quote-argument. The disadvantage is that this shell-quote-argument, basically rewritten from the equivalent Emacs code, is an unholy mess. It should really be "somebody else's problem". Using system* would make it such.

However, Knut's use of system* changes the (scm editor) APIs or rather ignores them and leaves them insecure, and there are uses in lily.scm as well.

So maybe we should change this to a system*-serving API? One way to do that would be to split into strings before replacing %(...)s style arguments, but that would mean that the command line stored in environment variables needs to get split with some method making use of the operating system conventions, and whether we mimic OS conventions when splitting or when quoting shell arguments does not really change the difficulty of the problem.

And if we find a problem in Guile 1.8.8's implementation of system*, we have a problem indeed.

Probably the safest method to make lilypond-invoke-editor's quoting "somebody else's problem" would be to rewrite in Python: this is the only Guile script I think, and Python has a maintained shell quote function in the pipes module.

But that would not help with lily.scm's use of (scm editor). I'm not even sure whether this use is dead code anyway. But if it isn't, biting the bullet and providing shell-quote-argument might actually be the best bet after all.

Attachments:

textedit.diff (4.4 kB; text/x-patch)

[issues:#5243] Fix security problem in lilypond-invoke-editor

Status: Started
Created: Thu Nov 23, 2017 08:35 AM UTC by Knut Petersen
Last Updated: Fri Nov 24, 2017 06:30 PM UTC
Owner: Knut Petersen

Fix security problem in lilypond-invoke-editor

If lilypond-invoke-editor was installed as a general
uri-helper it was easy to abuse it to execute arbitrary
code on an attacked system for non-textedit URIs.
This part of the problem was discovered and reported
to our bug-lilypond mailing list by Gabriel Corona.

But also pure textedit URIs were vulnerable, an
example is the URI

textedit:///:&xterm -e find ~/&:x:

that executes "find ~/" in a xterm.

With this patch lilypond-invoke-editor only
handles textedit URIs, and it does no longer
use the systems command processor but
guiles system* procedure for those URIs.

Also the script will abort if the line, char and
column fields of a textedit URI contain anything
but digits.

We could have fixed URI passing to the browser,
but it is not our job to provide a general URI helper.
Other software (e.g. xdg-open and friends) should
be used for that.

The security problem fixed now was introduced
into lilypond in the year 2005.

Signed-off-by: Knut Petersen address@hidden

http://codereview.appspot.com/336240043

Sent from sourceforge.net because address@hidden is subscribed to https://sourceforge.net/p/testlilyissues/issues/

To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/testlilyissues/admin/issues/options. Or, if this is a mailing list, you can unsubscribe from the mailing list.

From:	Auto mailings of changes to Lily Issues via Testlilyissues-auto
Subject:	[Lilypond-auto] [LilyIssues-auto] [testlilyissues:issues] #5243 Fix security problem in lilypond-invoke-editor
Date:	Fri, 24 Nov 2017 21:57:38 -0000