Subject: bug#77341: [PATCH] ; (find-function-search-for-symbol): Be cautious with macros.

From: Eshel Yaron <[email-masked]>
Date: Sat, 05 Apr 2025 08:29:31 +0200

close 77341 31.1
quit

Hi Daniel,

Daniel Colascione <[email-masked]> writes:

> On March 29, 2025 2:54:26 AM EDT, Eshel Yaron <[email-masked]> wrote:
>>Daniel Colascione <[email-masked]> writes:
>>
>>> On March 28, 2025 1:28:06 PM EDT, Eshel Yaron wrote:
>>>>Tags: patch
>>>>
>>>>find-function may expand Lisp macros in a source file when it fails to
>>>>find a definition otherwise.  This patch restricts this fallback to
>>>>trusted buffers only, to protect against possibly harmful macros.
>>>
>>> I get not wanting to execute code from random files I'm just visiting,
>>> but if I've already actually evaluated a macro function and installed
>>> it in my Emacs function namespace as something I can call, is it all
>>> that dangerous to call it? 
>>
>>find-function searches through code you haven't evaluated/loaded too.
>>Even for loaded libraries, the source file/buffer contents may be
>>different than the loaded code.  Either way, if you trust some files,
>>you can add them to trusted-content.  If you haven't, that means they
>>are untrusted.
>>
>>In general, as long as macro-expansion remains unsafe, we should avoid
>>expanding untrusted macros in commands that merely edit/browse Lisp code
>>(in contrast with compiling/evaluating it).
>>
>>> Instead of a blanket prohibition on macro expansion,
>>
>>(To be clear, I wouldn't say there's a prohibition on macro expansion,
>>just a restriction to trusted code, similarly to proper code evaluation,
>>since they're not that different in practice.)
>>
>>> I'd rather have macros declare that they're safe to run on untrusted
>>> inputs, which means mostly they don't eval their arguments.
>>
>>Yes, please :)
>
> Macros are just transformers from lists to lists. If a macro doesn't
> execute its input data, you can pass radioactive waste through it just
> fine.

Well, in practice macros can do literally anything at the moment,
including doing nasty things when given certain inputs, even without
executing the input directly...

> If we can trust a macro to handle untrusted data safely, we don't need
> to trust its inputs.

Agreed.

> It should work like this:
>
> 1) presumptively, expansion of a trusted form is safe no matter which macro is doing the expanding 
>
> 2) by default, expansion of untrusted forms is unsafe 
>
> 3) however, #2 can be overridden if a macro declares (probably via oclosure slot) "I am safely process untrusted
> input"
>
> Now we can expand most macros in practice even in untrusted code because most of the time the macros will be
> coming from the Emacs core, the macros of which we'll annotate as safe. No reason whatsoever that we shouldn't
> be able to expand, say, a cl-defgeneric form in an untrusted file.
>
> (We should use an oclosure instead of a plist property so we can easily attach the attribute to macrolet macros
> too)
>
> Probably simplest to make the safety annotation just another declare form. I'd love to integrate it into edebug
> specs, but sadly edebug doesn't have an API other components can use to extract and use the information embedded
> in debug specs, and current edebug syntax doesn't have all the information we'd need for safety. (Oh, this macro
> takes a sexp? I can still eval it during expansion!)

I wonder if unsafep could help with something like that.  It's not fully
developed/integrated, but ISTM to have the key notions in place, like
the side-effect-free property, that can be specified via declare forms.

>>Even better, we should have a safe evaluation sandbox that can be used
>>for safe macro-expansion among other things.  Indeed, any solution that
>>allows us to safely expand (most) macros would be a great improvement.
>
>>But until we have something like that, we should guard macro-expansion
>>behind trusted-content-p checks.
>
> I think we need a little more nuance than that

I'm not sure what you mean concretely.  As long as macro-expansion is
unrestrained (e.g. by something along the lines you've described above),
it's not safe to expand macros in untrusted code, so we should avoid it.
What nuance is missing?

Anyway, I suggest we start a separate discussion for general solutions
to safe macro-expansion -- regarding the specific case in find-function,
I've now pushed the patch, and with that, I'm closing this bug.

Thanks,

Eshel