Re: [POLL] We plan to remove #+LINK: ...%(my-function) placeholder from link abbreviation spec

2024-06-28 Thread Steven Allen
Suhail Singh  writes:

> Steven Allen  writes:
>
>> The concern is that, e.g., there may b a function _marked_ as pure
>> that's not actually pure, leaks some information, and/or has a
>> security vulnerability (e.g., a C function exposed to lisp that's
>> marked as pure but internally has, e.g., a buffer overflow).
>
> Are there any functions marked as pure, by default?
>

Yes. Any function that starts with:

(declare (pure t) ...

This flag was introduced to allow the byte/native compiler to better
optimize calls to pure functions. It's used here because "pure"
functions should be safe to call.



Re: [POLL] We plan to remove #+LINK: ...%(my-function) placeholder from link abbreviation spec

2024-06-28 Thread Steven Allen
Suhail Singh  writes:

> Steven Allen  writes:
>
>> 1. While this feature no longer invokes completely arbitrary code, it
>> still allows an attacker to call any function marked as "pure" which
>> is a pretty large attack surface.
>
> I am struggling to assess this, because it's not clear to me what the
> threat model is.  Could you please elaborate?  How are the attacker and
> potential victim interacting; what is the attack vector(s); who are the
> threat agents and what is their goal that we are trying to guard
> against, etc?

Scenario: Attacker sends an email containing an inline org-mode part with a
malicious link abbreviation.

The concern is that, e.g., there may b a function _marked_ as pure
that's not actually pure, leaks some information, and/or has a security
vulnerability (e.g., a C function exposed to lisp that's marked as pure
but internally has, e.g., a buffer overflow).

Of course, the actual attack hypothetical. The question being asked here
is: is the %(..) specifier in link abbreviations useful enough to warent
the potential risks.

>> You can, of course, write that function; but then you might as well
>> use org-link-abbrev-alist instead of defining a local #+LINK.
>
> Perhaps I misunderstood, I thought the thing being polled was whether or
> not to allow org-link-abbrev-alist to have REPLACE (per its docstring)
> be a function.  I.e., if %(my-function) is removed, so too would the
> ability to have a function in the REPLACE position in
> org-link-abbrev-alist.  Did I misunderstand?

The question is whether or not %(function) placeholders should be
allowed in #+LINK: lines. It doesn't actually say anything about
allowing them in the global org-link-abbrev-alist. But to be explicit,
there are three options:

1. Allow them in both #+LINK: lines and the global
org-link-abbrev-alist.

2. Allow them in org-link-abbrev-alist only.

3. Remove them entirely.



Re: [POLL] We plan to remove #+LINK: ...%(my-function) placeholder from link abbreviation spec

2024-06-28 Thread Steven Allen


Suhail Singh  writes:
> Ihor Radchenko  writes:
>
>> If you are actively using #+LINK: keywords with %(...) placeholders or
>> have any objections to this feature removal, please let us know.
>
> I do not actively use this feature, however, removing it seems
> excessive.  IIUC, it's a useful feature in situations when the tag may
> require deterministic, yet non-trivial manipulation.  The current
> mechanism of restricting this to functions marked safe by user seems
> sufficient.
>
> Am I missing something?  Is the threat model such that it can only be
> adequately addressed by removing the feature altogether?

There are two issues:

1. While this feature no longer invokes completely arbitrary code, it
still allows an attacker to call any function marked as "pure" which is
a pretty large attack surface.

2. Making it secure also made it significantly less useful, if it ever
was all that useful. For the %(...) specifier to be useful, you need a
pure/safe function that takes exactly one string argument and produces
the string you need. You can, of course, write that function; but then
you might as well use org-link-abbrev-alist instead of defining a
local #+LINK.

Personally, I'd start by forbidding %(...) placeholders in buffer-local
#+LINK: definitions, they're perfectly safe in org-link-abbrev-alist.



Re: [POLL] Bug of Feature? Attack vector via deceiving link abbrevs (was: [ANN] Emergency bugfix release: Org mode 9.7.5)

2024-06-28 Thread Steven Allen
Ihor Radchenko  writes:

> Ihor Radchenko  writes:
>
>> I just released Org mode 9.7.5 that fixes a critical vulnerability.
>> The release is coordinated with emergency Emacs 29.4 release.
>
> This one is another potential issue (or a feature) we have found while
> discussing the main vulnerability.
>
> Currently, one can create an Org file like
>
> #+LINK: https https://fake-gmail-login-page.xyz/
> [[https://gmail.com]]

This is no different from:

[[https://fake-gmail-login-page.xyz][https://gmail.com]]

In both cases, mousing over the link will show you the actual target address.

On the other hand, having different faces for "plain" links (links where
the text in the buffer matches the link target) and special links would
be kind of nice.



Re: [ANN] Emergency bugfix release: Org mode 9.7.5

2024-06-22 Thread Steven Allen
Greg Troxel  writes:

> (Thanks for fixing and your efforts on org.  I've been an org user since
> at least July of 2010.)
>
> Just to be clear, is this the commit that needs applying to emacs
> sources, 29.3, 28.x, and so on?

Yes, that's the correct commit.

> It seems so, but I would rather not guess. I'm asking on behalf of
> pkgsrc, where I am managing the release process for our 2024Q2 branch,
> due on 30 June. Believe it or not we have 20, 21, 26, 27, 28, 29 and a
> from-git version. While some should be pruned, some people use it on
> vaxes. Any idea how far back this goes?

It was introduced in org 7.9 (commit [1] from July of 2012). From what I
can tell, it has been present in Emacs since emacs-24.2.

[1]: ef3d4b5965b828e85a535ef3f32999473c6a2a7a 

>
> Thanks,
> Greg
>
> commit f4cc61636947b5c2f0afc67174dd369fe3277aa8
> Author: Ihor Radchenko 
> Date:   Tue Jun 18 13:06:44 2024 +0200
>
> org-link-expand-abbrev: Do not evaluate arbitrary unsafe Elisp code
> 
> * lisp/ol.el (org-link-expand-abbrev): Refuse expanding %(...) link
> abbrevs that specify unsafe function.  Instead, display a warning, and
> do not expand the abbrev.  Clear all the text properties from the
> returned link, to avoid any potential vulnerabilities caused by
> properties that may contain arbitrary Elisp.
>
> diff --git a/lisp/ol.el b/lisp/ol.el
> index 7a7f4f558..8a556c7b9 100644
> --- a/lisp/ol.el
> +++ b/lisp/ol.el
> @@ -1152,17 +1152,35 @@ Abbreviations are defined in `org-link-abbrev-alist'."
>(if (not as)
> link
>   (setq rpl (cdr as))
> - (cond
> -  ((symbolp rpl) (funcall rpl tag))
> -  ((string-match "%(\\([^)]+\\))" rpl)
> -   (replace-match
> -(save-match-data
> -  (funcall (intern-soft (match-string 1 rpl)) tag))
> -t t rpl))
> -  ((string-match "%s" rpl) (replace-match (or tag "") t t rpl))
> -  ((string-match "%h" rpl)
> -   (replace-match (url-hexify-string (or tag "")) t t rpl))
> -  (t (concat rpl tag)))
> +;; Drop any potentially dangerous text properties like
> +;; `modification-hooks' that may be used as an attack vector.
> +(substring-no-properties
> +  (cond
> +   ((symbolp rpl) (funcall rpl tag))
> +   ((string-match "%(\\([^)]+\\))" rpl)
> +   (let ((rpl-fun-symbol (intern-soft (match-string 1 rpl
> + ;; Using `unsafep-function' is not quite enough because
> + ;; Emacs considers functions like `genenv' safe, while
> + ;; they can potentially be used to expose private system
> + ;; data to attacker if abbreviated link is clicked.
> + (if (or (eq t (get rpl-fun-symbol 'org-link-abbrev-safe))
> + (eq t (get rpl-fun-symbol 'pure)))
> + (replace-match
> +   (save-match-data
> + (funcall (intern-soft (match-string 1 rpl)) tag))
> +   t t rpl)
> +   (org-display-warning
> +(format "Disabling unsafe link abbrev: %s
> +You may mark function safe via (put '%s 'org-link-abbrev-safe t)"
> +rpl (match-string 1 rpl)))
> +   (setq org-link-abbrev-alist-local (delete as 
> org-link-abbrev-alist-local)
> + org-link-abbrev-alist (delete as org-link-abbrev-alist))
> +   link
> +)))
> +   ((string-match "%s" rpl) (replace-match (or tag "") t t rpl))
> +   ((string-match "%h" rpl)
> +(replace-match (url-hexify-string (or tag "")) t t rpl))
> +   (t (concat rpl tag
>  
>  (defun org-link-open (link  arg)
>"Open a link object LINK.



Re: [PATCH] org.el: inline display of attached images in link descriptions

2023-06-02 Thread Steven Allen

You're right and now I have no idea what I was testing. I'm
apologize for that.

The attached patch should actually work this time. It's not great (setq
in a let), but anything else would likely be a much larger change.

>From 6ca24ca705fc3856c68957cfa50dfce77c9148c9 Mon Sep 17 00:00:00 2001
From: Steven Allen 
Date: Thu, 1 Jun 2023 11:49:19 -0700
Subject: [PATCH] org.el: inline display of attached images in link
 descriptions

* lisp/org.el (org-display-inline-images): inline display of attached
images in link descriptions.

Previously, `org-display-inline-images' only inlined images in link
descriptions when they were explicit "file:" links. This change adds
support for "attachment:" links. E.g.:

[[https://orgmode.org][attachment:emacs-screenshot.png]]
---
 lisp/org.el | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 0e5740412..405cae9a5 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -16301,7 +16301,8 @@ defun org-create-formula-image
 
   2. Its description consists in a single link of the previous
  type.  In this case, that link must be a well-formed plain
- or angle link, i.e., it must have an explicit \"file\" type.
+ or angle link, i.e., it must have an explicit \"file\" or
+ \"attachment\" type.
 
 Equip each image with the key-map `image-map'.
 
@@ -16332,7 +16333,7 @@ defun org-create-formula-image
 	   ;; "file:" links.  Also check link abbreviations since
 	   ;; some might expand to "file" links.
 	   (file-types-re
-		(format "\\[\\[\\(?:file%s:\\|attachment:\\|[./~]\\)\\|\\]\\[\\(

[PATCH] org.el: inline display of attached images in link descriptions

2023-06-01 Thread Steven Allen
* lisp/org.el (org-display-inline-images): inline display of attached
images in link descriptions.

Previously, `org-display-inline-images' only inlined images in link
descriptions when they were explicit "file:" links. This change adds
support for "attachment:" links. E.g.:

[[https://orgmode.org][attachment:emacs-screenshot.png]]
---
 lisp/org.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 0e5740412..3e1b757c4 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -16301,7 +16301,8 @@ defun org-create-formula-image
 
   2. Its description consists in a single link of the previous
  type.  In this case, that link must be a well-formed plain
- or angle link, i.e., it must have an explicit \"file\" type.
+ or angle link, i.e., it must have an explicit \"file\" or
+ \"attachment\" type.
 
 Equip each image with the key-map `image-map'.
 
@@ -16332,7 +16333,7 @@ defun org-create-formula-image
   ;; "file:" links.  Also check link abbreviations since
   ;; some might expand to "file" links.
   (file-types-re
-   (format 
"\\[\\[\\(?:file%s:\\|attachment:\\|[./~]\\)\\|\\]\\[\\(