Re: Security problem: lilypond-invoke-editor

[David Wright]

On Thu 23 Nov 2017 at 10:27:36 (+0000), J Martin Rushton wrote:
> On 23/11/17 09:23, David Kastrup wrote:
> > Knut Petersen <address@hidden> writes:
> > 
> >> 12 years ago a security problem was introduced into lilypond-invoke-editor.
> >> On 2017/11/15 the problem was reported to the bug-lilypond mailing
> >> list by Gabriel Corona.
> > 
> > [...]
> > 
> >> If you do not know if you are affected:
> >>
> >> 1.: locate lilypond-invoke-editor
> >>
> >> 2. Open lilypond-invoke-editor in your favorite text editor. Search for
> >>
> >>        (if (is-textedit-uri? uri)
> >>          (run-editor uri)
> >>          (run-browser uri)))))
> >>
> >> and replace it with
> >>
> >>        (if (is-textedit-uri? uri)
> >>          (run-editor uri)))))
> > 
> > Stupid question: what does run-editor do to be inherently safer than
> > run-browser, and what would prevent run-browser from doing the same?
> > 
> > The reason I am asking is that changing the semantics significantly
> > before 2.20 is icky, yet we would not want to leave a security hole
> > around we have been given notice of.
> > 
> > So the question is whether there would not be a sort-of trivial patchup
> > of this preserving the original intent.
> > 
> > For the long haul, it's probably the right fix on GNU/Linux systems.  I
> > just have no idea how this would affect other systems and possibly our
> > installers.
> > 
> Just to make life hard, using "command -v lilypond-invoke-editor" turns
> up a file in /usr/local/bin.  It is a symbolic link to
> /usr/local/bin/lilypond-wrapper.guile.  That file is (truncating to
> avoid wrapping):
> 
> #!/bin/sh
> export PYTHONPATH= ...
> export GUILE_LOAD_PATH= ...
> export LD_LIBRARY_PATH= ...
> me=`basename $0`
> exec "/usr/local/lilypond/usr/bin/guile"  \
>   -e main "/usr/local/lilypond/usr/bin/$me" "$@"
> 
> It is the file /usr/local/lilypond/usr/bin/lilypond-invoke-editor which
> contains the statements above.

Of course, this file could be located at
<--prefix>/lilypond/usr/bin/lilypond-invoke-editor
if you install multiple versions/install as a user/use a --prefix as root.

What I'm wondering about, though, is why the definition of run-browser
has a hard-coded reference to "firefox". If $BROWSER is unset, surely
this merits an error failure in the same way as run-editor just above,
rather than calling a potentially insecure binary without so much as
a by your leave. AFAICT this will bypass, for example, the security
checks/sanitisation made by Debian systems using the "sensible-browser"
binary to invoke the browser. The Debian package sensible-utils has
already been fixed.
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=881767

Cheers,
David.