bug#44018: Don't consider play-sound-file to be a 'safe' function

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Mattias Engdegård]

We should remove play-sound-file from the list of 'safe' functions in unsafep.el.
The risks outweigh the benefits here; this is just basic security engineering.
The attack surface of play-sound-file is considerable.

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Lars Ingebrigtsen]

Mattias Engdegård <mattiase@acm.org> writes:

> We should remove play-sound-file from the list of 'safe' functions in
> unsafep.el.
> The risks outweigh the benefits here; this is just basic security engineering.
> The attack surface of play-sound-file is considerable.

Makes sense to me; go ahead.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Thu, 15 Oct 2020 18:55:26 +0200
> 
> We should remove play-sound-file from the list of 'safe' functions in unsafep.el.
> The risks outweigh the benefits here; this is just basic security engineering.
> The attack surface of play-sound-file is considerable.

Any details for the uninitiated, or pointers to the info?

Are the risks the same on all the supported platforms, or just on
some?

Thanks.

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Mattias Engdegård]

15 okt. 2020 kl. 19.26 skrev Eli Zaretskii <eliz@gnu.org>:

> Any details for the uninitiated, or pointers to the info?

You are definitely not uninitiated but others may be so please bear with me.

There are many things that can go wrong:

Playing sound files involves lots of code and libraries, sometimes even executing external processes.
Sound file formats are complex and a player typically needs to understand several different ones; security-related bugs are not uncommon.
Sound file players may also need access to the hardware, which can greatly amplify the severity of any breach.

> Are the risks the same on all the supported platforms, or just on
> some?

The security fundamentals (as above) are the same everywhere; details obviously differ. Even if we could pronounce one platform as entirely 'safe' for audio-playing, which I don't think is feasible, I don't see the gain from doing so.

Obviously 'safe' has to be understood in context. Can Emacs be tricked to call play-sound-file with the name of a crafted file as argument? Maybe; as far as I can tell, unsafe is only used by SES in Emacs proper, but it seems feasible to create a .ses file that calls play-sound-file without asking the user. To assume otherwise would be imprudent.

It is true that the hostile Internet has hardened audio file code considerably over the years but why would we explicitly make a security exception for a function with large attack surface in an application (Emacs) that may very well be used for inspection of potentially harmful files?

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Thu, 15 Oct 2020 21:01:20 +0200
> Cc: 44018@debbugs.gnu.org
> 
> 15 okt. 2020 kl. 19.26 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> > Any details for the uninitiated, or pointers to the info?
> 
> You are definitely not uninitiated but others may be so please bear with me.
> 
> There are many things that can go wrong:
> 
> Playing sound files involves lots of code and libraries, sometimes even executing external processes.
> Sound file formats are complex and a player typically needs to understand several different ones; security-related bugs are not uncommon.
> Sound file players may also need access to the hardware, which can greatly amplify the severity of any breach.

Any specifics, though?  Surely, if the risks are known, there should
be some vulnerabilities recorded somewhere?  Is it possible to give a
couple of examples, or refer to known vulnerabilities?

> > Are the risks the same on all the supported platforms, or just on
> > some?
> 
> The security fundamentals (as above) are the same everywhere; details obviously differ. Even if we could pronounce one platform as entirely 'safe' for audio-playing, which I don't think is feasible, I don't see the gain from doing so.

I asked because I looked for any known security risks associated with
the MCI interface we use on MS-Windows to implement
play-sound-internal, and couldn't find any.  Maybe I overlooked
something, or used sub-optimal search phrases, so I'd love to see
something about the risks.  Otherwise it sounds (pun intended) like we
are afraid of a danger that doesn't exist.

> Obviously 'safe' has to be understood in context. Can Emacs be tricked to call play-sound-file with the name of a crafted file as argument? Maybe; as far as I can tell, unsafe is only used by SES in Emacs proper, but it seems feasible to create a .ses file that calls play-sound-file without asking the user. To assume otherwise would be imprudent.

I'm not sure I understand: what's unsafe in playing sound?  I thought
you were talking about the danger of using a malicious file that is
disguised as a sound file, but in that case the fact that we invoke
the function is not the problem, the problem would be (AFAIU) if the
sound device failed to recognize the file as corrupted or of wrong
format, and caused whatever damage that was supposed to do.  Those are
the kind of details about the vulnerabilities I expected to see.  Is
any information along these lines available?

> It is true that the hostile Internet has hardened audio file code considerably over the years but why would we explicitly make a security exception for a function with large attack surface in an application (Emacs) that may very well be used for inspection of potentially harmful files?

I'm struggling to understand what is the attack surface.  Can you (or
someone else) help in understanding that?

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Mattias Engdegård]

15 okt. 2020 kl. 21.20 skrev Eli Zaretskii <eliz@gnu.org>:

> Any specifics, though?  Surely, if the risks are known, there should
> be some vulnerabilities recorded somewhere?  Is it possible to give a
> couple of examples, or refer to known vulnerabilities?

Sorry for being unclear. This is not about known vulnerabilities (which we hope are not present!) but about reducing risks -- ie, exposure to unknown present and future security holes.

Security thinking takes some getting used to; risks have to be motivated by clear needs. In this case, the needs for calls to play-sound-file with arbitrary input data seem very small but the risks aren't commensurate.

> I'm not sure I understand: what's unsafe in playing sound?

A fair question! Safe parsing and handling of binary file formats can be surprisingly difficult, especially in an unsafe language; the slightest mistake can be exploited by an intelligent adversary. It could be a buffer overrun, an unsufficiently checked parameter, signedness confusion, nonsensical combination of values, unexpected alignment, legal but untested values, text encoding traps in string fields (oh yes)... I have written many sound and image file parsers over the years, most of which would be completely unsuitable in a hostile environment.

And new attack vectors keep turning up. Inaudible sound streams carrying covert instructions to always-on voice-activated assistants? Maybe all they can do is to turn off the lights or order you more rolls of toilet paper but don't count on it.

Most weaknesses are likely to have been fuzzed and patched out of existence by now but it does not change the fact that playing arbitrary audio is an activity that will always carry more inherent risk than, say, calling ding or split-string, also in the same list of safe functions.

But, someone objects, I play arbitrary sound files in my web browser all the time! Yes, and that is not free of risk either -- but that web browser is likely to have undergone more care and hardening of these code paths than Emacs: more careful selection of libraries and APIs used, fewer obscure or obscure file formats supported, and even measures taken to contain breaches, such as sandboxing.

Finally, Emacs must be completely trusted as a tool for inspecting arbitrary files, even ones prepared with malicious intent.

> Do you understand why 'message' was removed?

I can only guess, but that function could be used to display deceptive messages that mislead the user to take actions against his or her own interest. Removing it looks like a wise decision.

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Mattias Engdegård]

Given Lars's approval and the lack of further objections, I'm removing play-sound-file from the list of safe functions in unsafep.el. The change is easily reverted if there are misgivings.

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Mon, 26 Oct 2020 12:51:22 +0100
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, Eli Zaretskii <eliz@gnu.org>
> 
> Given Lars's approval and the lack of further objections, I'm removing play-sound-file from the list of safe functions in unsafep.el.

I guess you only count opinions and arguments of those who agree with
you, sigh.

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Mattias Engdegård]

reopen 44018
stop

26 okt. 2020 kl. 16.29 skrev Eli Zaretskii <eliz@gnu.org>:

>> Given Lars's approval and the lack of further objections, I'm removing play-sound-file from the list of safe functions in unsafep.el.
> 
> I guess you only count opinions and arguments of those who agree with
> you, sigh.

You didn't object to my explanation, and I explicitly offered to revert the change if any objection would come forth. Since you now have effectively objected, I'm making good of my promise.

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Mon, 26 Oct 2020 17:19:52 +0100
> Cc: 44018@debbugs.gnu.org, larsi@gnus.org
> 
> >> Given Lars's approval and the lack of further objections, I'm removing play-sound-file from the list of safe functions in unsafep.el.
> > 
> > I guess you only count opinions and arguments of those who agree with
> > you, sigh.
> 
> You didn't object to my explanation, and I explicitly offered to revert the change if any objection would come forth. Since you now have effectively objected, I'm making good of my promise.

Thank you.

I don't "object" object, I just prefer to end the discussion and reach
some conclusions before "shots are fired".  I'd like to hear Lars's
opinions on the questions I asked in my last message.

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> I don't "object" object, I just prefer to end the discussion and reach
> some conclusions before "shots are fired".  I'd like to hear Lars's
> opinions on the questions I asked in my last message.

I didn't really have any opinions beyond what I already said -- that
reading the code and looking and the changes, it seemed to me that the
purpose of unsafep.el was to list functions that are absolutely 100%
"safe" to use (i.e., no very noticeable side effects).

So the logic seems to be "kinda like side-effect-free, but from a user
point of view".  And neither message nor play-sound-file fit those
criteria.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Stefan Kangas]

Mattias Engdegård <mattiase@acm.org> writes:

> Given Lars's approval and the lack of further objections, I'm removing play-sound-file from the list of safe functions in unsafep.el.

FWIW, I agree with this change.  There have been many vulnerabilities
in this area, for example in the Linux kernel drivers.  Who knows what
else is out there.

See, for example:
https://nvd.nist.gov/vuln/search/results?form_type=Basic&results_type=overview&query=linux+audio&search_type=all

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Mattias Engdegård]

reopen 44018
stop

[Stefan, I think you accidentally closed the bug when replying to 44018-done@debbugs.gnu.org. I would probably have made the same mistake!]

26 okt. 2020 kl. 17.32 skrev Stefan Kangas <stefan@marxist.se>:

> FWIW, I agree with this change.  There have been many vulnerabilities
> in this area, for example in the Linux kernel drivers.  Who knows what
> else is out there.

Yes, and some of us have dealt with these things for decades.
My fav audio bug in Linux is the famous gstreamer NES emu exploit [1], which is a good example of what I tried to explain about attack surfaces and unknown dangers.

[1] https://scarybeastsecurity.blogspot.com/2016/11/0day-exploit-compromising-linux-desktop.html

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Are the risks the same on all the supported platforms, or just on
> some?

My understanding of unsafep.el isn't that it's trying to protect against
any particular exploits, but just give a list of things that are totally
and utterly OK to eval.  So you have stuff like:

commit a8c41b4c0d3b0a3e87f17bbcdd8ac12dae296b3a
Author:     Chong Yidong <cyd@stupidchicken.com>
AuthorDate: Mon Oct 18 13:28:20 2010 -0400

    Don't allow functions that display messages in unsafep.

So even `message' isn't "safe" in this context.  I think it's odd to
have `play-sound-file' marked as "safe" if `message' isn't.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Mattias Engdegård <mattiase@acm.org>,
>   44018@debbugs.gnu.org
> Date: Fri, 16 Oct 2020 07:39:05 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Are the risks the same on all the supported platforms, or just on
> > some?
> 
> My understanding of unsafep.el isn't that it's trying to protect against
> any particular exploits, but just give a list of things that are totally
> and utterly OK to eval.  So you have stuff like:
> 
> commit a8c41b4c0d3b0a3e87f17bbcdd8ac12dae296b3a
> Author:     Chong Yidong <cyd@stupidchicken.com>
> AuthorDate: Mon Oct 18 13:28:20 2010 -0400
> 
>     Don't allow functions that display messages in unsafep.
> 
> So even `message' isn't "safe" in this context.  I think it's odd to
> have `play-sound-file' marked as "safe" if `message' isn't.

Do you understand why 'message' was removed?  I don't, and couldn't
find any discussion on Emacs lists that discussed that; I may have
missed something.  I have no idea why 'message' could be unsafe.
unsafep.el doesn't provide a high-level definition of what is
considered "safe", unfortunately, and was evidently written for SES,
so may have some bias due to that context.  Still, it is not clear to
me why 'message' was removed.

I'm uneasy with doing things when the only argument is "why not?".
Maybe I'm the odd one out, but I generally think we should have a lot
of respect for those who wrote code for Emacs in the past, unless we
have a clear reason to think it was in error of some kind.  So I'm
trying to get to the bottom of an issue when the proposal is clearly
at odds with something we had for years.

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Basil L. Contovounesios]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Lars Ingebrigtsen <larsi@gnus.org>
>> Cc: Mattias Engdegård <mattiase@acm.org>,
>>   44018@debbugs.gnu.org
>> Date: Fri, 16 Oct 2020 07:39:05 +0200
>> 
>> My understanding of unsafep.el isn't that it's trying to protect against
>> any particular exploits, but just give a list of things that are totally
>> and utterly OK to eval.  So you have stuff like:
>> 
>> commit a8c41b4c0d3b0a3e87f17bbcdd8ac12dae296b3a
>> Author:     Chong Yidong <cyd@stupidchicken.com>
>> AuthorDate: Mon Oct 18 13:28:20 2010 -0400
>> 
>>     Don't allow functions that display messages in unsafep.
>> 
>> So even `message' isn't "safe" in this context.  I think it's odd to
>> have `play-sound-file' marked as "safe" if `message' isn't.
>
> Do you understand why 'message' was removed?  I don't, and couldn't
> find any discussion on Emacs lists that discussed that; I may have
> missed something.  I have no idea why 'message' could be unsafe.
> unsafep.el doesn't provide a high-level definition of what is
> considered "safe", unfortunately, and was evidently written for SES,
> so may have some bias due to that context.  Still, it is not clear to
> me why 'message' was removed.

FWIW, there's an @ignored section in doc/lispref/functions.texi that I
guess was intended to provide a higher-level description, but the
following paragraph is the best it currently manages to do:

  What is a safe Lisp expression?  Basically, it's an expression that
  calls only built-in functions with no side effects (or only innocuous
  ones).  Innocuous side effects include displaying messages and
  altering non-risky buffer-local variables (but not global variables).

-- 
Basil

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>,  mattiase@acm.org,
>   44018@debbugs.gnu.org
> Date: Mon, 26 Oct 2020 17:05:56 +0000
> 
> > Do you understand why 'message' was removed?  I don't, and couldn't
> > find any discussion on Emacs lists that discussed that; I may have
> > missed something.  I have no idea why 'message' could be unsafe.
> > unsafep.el doesn't provide a high-level definition of what is
> > considered "safe", unfortunately, and was evidently written for SES,
> > so may have some bias due to that context.  Still, it is not clear to
> > me why 'message' was removed.
> 
> FWIW, there's an @ignored section in doc/lispref/functions.texi that I
> guess was intended to provide a higher-level description, but the
> following paragraph is the best it currently manages to do:
> 
>   What is a safe Lisp expression?  Basically, it's an expression that
>   calls only built-in functions with no side effects (or only innocuous
>   ones).  Innocuous side effects include displaying messages and
>   altering non-risky buffer-local variables (but not global variables).

Thanks.  But this seems to say 'message' should be considered safe?

Anyway, if this is the criterion we want to apply, let's document it
in unsafep.el, and let's examine the list there with this definition
in mind.

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Mattias Engdegård]

26 okt. 2020 kl. 18.16 skrev Eli Zaretskii <eliz@gnu.org>:

>>  What is a safe Lisp expression?  Basically, it's an expression that
>>  calls only built-in functions with no side effects (or only innocuous
>>  ones).  Innocuous side effects include displaying messages and
>>  altering non-risky buffer-local variables (but not global variables).
> 
> Thanks.  But this seems to say 'message' should be considered safe?

What counted as 'innocuous' back then may not do so today.
Freely displaying messages to the user as if they came from a trusted system isn't necessarily harmless by modern standards of security.

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Mon, 26 Oct 2020 18:38:07 +0100
> Cc: "Basil L. Contovounesios" <contovob@tcd.ie>, larsi@gnus.org,
>         44018@debbugs.gnu.org
> 
> >>  What is a safe Lisp expression?  Basically, it's an expression that
> >>  calls only built-in functions with no side effects (or only innocuous
> >>  ones).  Innocuous side effects include displaying messages and
> >>  altering non-risky buffer-local variables (but not global variables).
> > 
> > Thanks.  But this seems to say 'message' should be considered safe?
> 
> What counted as 'innocuous' back then may not do so today.
> Freely displaying messages to the user as if they came from a trusted system isn't necessarily harmless by modern standards of security.

So you are saying 'message' is not safe because it could be used to
display text that isn't "innocuous"?  In that case, we should also
remove from the list 'error' and 'signal', no?

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Mattias Engdegård]

26 okt. 2020 kl. 19.28 skrev Eli Zaretskii <eliz@gnu.org>:

> So you are saying 'message' is not safe because it could be used to
> display text that isn't "innocuous"?  In that case, we should also
> remove from the list 'error' and 'signal', no?

That's a good point. Perhaps we should.
There doesn't seem to be much point in permitting 'while' either in the absence of mutation, but I don't think it's actively harmful. I could be wrong.

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Mon, 26 Oct 2020 21:36:50 +0100
> Cc: contovob@tcd.ie, larsi@gnus.org, 44018@debbugs.gnu.org
> 
> 26 okt. 2020 kl. 19.28 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> > So you are saying 'message' is not safe because it could be used to
> > display text that isn't "innocuous"?  In that case, we should also
> > remove from the list 'error' and 'signal', no?
> 
> That's a good point. Perhaps we should.
> There doesn't seem to be much point in permitting 'while' either in the absence of mutation, but I don't think it's actively harmful. I could be wrong.

I'm okay with doing this, but please add commentary that explains
these removals and the general policy for considering a function
"safe".

Thanks.

bug#44018: Don't consider play-sound-file to be a 'safe' function

[Mattias Engdegård]

31 okt. 2020 kl. 09.06 skrev Eli Zaretskii <eliz@gnu.org>:

> I'm okay with doing this, but please add commentary that explains
> these removals and the general policy for considering a function
> "safe".

Agreed -- done and pushed.

I also removed 'throw' since it can be used to alter the flow control of a program if evaluated inside a 'catch' form as well as inject an arbitrary value that way, and 'catch' since it makes little sense without 'throw'.
More importantly, 'replace-regexp-in-string' was removed because it could be used to execute arbitrary code.
I've removed the side-effect-free property of 'assoc' for the same reason.

These mistakes, and the fact that unsafep allows some limited use of setq, apply, let, push, pop etc, makes me doubt the safety of unsafep entirely and would recommend against using it in new code. The attack surface is simply too large and difficult to overview. For example, the long list of 'side-effect-free' functions hasn't really been vetted with security in mind (assoc is a case in point).