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](https://sourceforge.net/p/testlilyissues/issues/_discuss/thread/3c894c15/7c44/attachment/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 <[email protected]>
http://codereview.appspot.com/336240043
---
Sent from sourceforge.net because [email protected] 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.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Testlilyissues-auto mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/testlilyissues-auto
