Subject: Re: bug#28350: enriched.el code execution

From: [email-masked] (Charles A. Roelli)
Date: Sat, 09 Sep 2017 14:23:54 +0200

> Date: Thu, 07 Sep 2017 05:34:34 +0300
> From: Eli Zaretskii <[email-masked]>
> CC: [email-masked]
> 
> > Date: Wed, 06 Sep 2017 21:25:18 +0200
> > From: [email-masked] (Charles A. Roelli)
> > 
> > As for a fix to apply to master: I'd like to keep "x-display" if we
> > can agree on some "safe" predicate that the given parameter would have
> > to satisfy.  Looking at the list of display specifications that are
> > available, it seems that simple string, margin text, space-width,
> > height (only in the (+ n), (- n) and n cases) and raise specifications
> > should be okay.  Does anybody else have an opinion about this?
> 
> I agree that the cases you have shown are safe.
> 
> Thanks.

Thank you.  Does the attached patch look OK?  I've used the file
enriched-test-safe-props.txt (also attached) to test that safe
properties are still applied.

>From 1c58b3e76a80a342c2f7e96d91214fe49678f471 Mon Sep 17 00:00:00 2001
From: "Charles A. Roelli" <[email-masked]>
Date: Sat, 9 Sep 2017 14:03:58 +0200
Subject: [PATCH] Prevent code execution by text/enriched files (Bug#28350)

* lisp/textmodes/enriched.el (enriched-display-prop-safe-p): New
function.
(enriched-decode-display-prop): Use it to prevent unsafe display
properties from being applied.
---
 lisp/textmodes/enriched.el | 43 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/lisp/textmodes/enriched.el b/lisp/textmodes/enriched.el
index 7ace2a5..f496259 100644
--- a/lisp/textmodes/enriched.el
+++ b/lisp/textmodes/enriched.el
@@ -503,6 +503,47 @@ enriched-decode-display-prop
 		  (error nil)))))
     (unless prop
       (message "Warning: invalid <x-display> parameter %s" param))
-    (list start end 'display prop)))
+    (if (enriched-display-prop-safe-p prop)
+        (list start end 'display prop)
+      (message "Warning: unsafe <x-display> parameter %s not applied" param)
+      (list start end))))
+
+(defun enriched-display-prop-safe-p (prop)
+  "Return t if display property PROP is safe to apply to text.
+
+A safe display property is either:
+
+  - a string,
+
+  - a space-width display specification, (space-width factor),
+    where FACTOR is an integer or a float,
+
+  - a margin display specification, ((margin right-margin) spec)
+    or ((margin left-margin) spec), where SPEC is a string,
+
+  - a height display specification, (height spec), where SPEC is
+    of the form (+ n), (- n) or n, and N is an integer,
+
+  - or a raise display specification, (raise factor), where
+    FACTOR is an integer.
+
+See Info node `(elisp)Display Property' for the use of these
+display specifications."
+  (ignore-errors
+    (or (stringp prop)
+        (and (eq (car prop) 'space-width)
+             (or (integerp (cadr prop)) (floatp (cadr prop))))
+        (and (consp (car prop))
+             (eq (caar prop) 'margin)
+             (or (eq (cadar prop) 'right-margin)
+                 (eq (cadar prop) 'left-margin))
+             (stringp (cadr prop)))
+        (and (eq (car prop) 'height)
+             (or (integerp (cadr prop))
+                 (and (listp (cadr prop))
+                      (or (eq (elt (cadr prop) 0) '+) (elt (cadr prop) 0) '-)
+                      (integerp (elt (cadr prop) 1)))))
+        (and (eq (car prop) 'raise)
+             (integerp (cadr prop))))))
 
 ;;; enriched.el ends here
-- 
2.9.4