From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: charles@aurox.ch (Charles A. Roelli) Newsgroups: gmane.emacs.bugs Subject: bug#28350: enriched.el code execution Date: Sat, 09 Sep 2017 22:37:29 +0200 Message-ID: References: <837exb1bk5.fsf@gnu.org> <838thovvcr.fsf@gnu.org> <83wp57vmk6.fsf@gnu.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: blaine.gmane.org 1504989566 24458 195.159.176.226 (9 Sep 2017 20:39:26 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sat, 9 Sep 2017 20:39:26 +0000 (UTC) Cc: 28350@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sat Sep 09 22:39:22 2017 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dqmWv-0005Q5-P8 for geb-bug-gnu-emacs@m.gmane.org; Sat, 09 Sep 2017 22:39:06 +0200 Original-Received: from localhost ([::1]:50817 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dqmX2-0003UW-Ve for geb-bug-gnu-emacs@m.gmane.org; Sat, 09 Sep 2017 16:39:13 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:46734) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dqmWv-0003St-UV for bug-gnu-emacs@gnu.org; Sat, 09 Sep 2017 16:39:07 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dqmWs-0002YM-PG for bug-gnu-emacs@gnu.org; Sat, 09 Sep 2017 16:39:05 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:49271) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dqmWs-0002Y5-LZ for bug-gnu-emacs@gnu.org; Sat, 09 Sep 2017 16:39:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1dqmWs-00076O-Fa for bug-gnu-emacs@gnu.org; Sat, 09 Sep 2017 16:39:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: charles@aurox.ch (Charles A. Roelli) Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 09 Sep 2017 20:39:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 28350 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: security Original-Received: via spool by 28350-submit@debbugs.gnu.org id=B28350.150498949327244 (code B ref 28350); Sat, 09 Sep 2017 20:39:02 +0000 Original-Received: (at 28350) by debbugs.gnu.org; 9 Sep 2017 20:38:13 +0000 Original-Received: from localhost ([127.0.0.1]:57950 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dqmW4-00075L-MT for submit@debbugs.gnu.org; Sat, 09 Sep 2017 16:38:13 -0400 Original-Received: from sinyavsky.aurox.ch ([37.35.109.145]:55040) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dqmW2-000757-SX for 28350@debbugs.gnu.org; Sat, 09 Sep 2017 16:38:11 -0400 Original-Received: from sinyavsky.aurox.ch (sinyavsky.aurox.ch [127.0.0.1]) by sinyavsky.aurox.ch (Postfix) with ESMTP id B490B22532 for <28350@debbugs.gnu.org>; Sat, 9 Sep 2017 20:31:57 +0000 (UTC) Authentication-Results: sinyavsky.aurox.ch (amavisd-new); dkim=pass (1024-bit key) reason="pass (just generated, assumed good)" header.d=aurox.ch DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=aurox.ch; h= content-type:content-type:mime-version:references:subject :subject:in-reply-to:to:from:from:message-id:date:date; s=dkim; t=1504989114; x=1505853115; bh=OPJLqAjvJrdiEmME4e3oIE6iv4wYmeeK r89QdQVFseQ=; b=Z/GAe8e+09tI+Jj7Ahi8dy/wOKfmhOtynX2RG85dXNNuKBEr jE0zl+wtqvHDc87EFNF0MHWlOvK0wlUjB1hLAK7KLfSxwkQlbfHVpVEdL7XGv2r2 oK69xFkaaYE9hcH67ZCDLojij7ZMFGPIP+IhnaFGSBH2FcwAR2T+hsMOs+w= X-Virus-Scanned: Debian amavisd-new at test.virtualizor.com Original-Received: from sinyavsky.aurox.ch ([127.0.0.1]) by sinyavsky.aurox.ch (sinyavsky.aurox.ch [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id U0Te3bPCBDG3 for <28350@debbugs.gnu.org>; Sat, 9 Sep 2017 20:31:54 +0000 (UTC) Original-Received: from gray (125.85.192.178.dynamic.wline.res.cust.swisscom.ch [178.192.85.125]) by sinyavsky.aurox.ch (Postfix) with ESMTPSA id 943B622529; Sat, 9 Sep 2017 20:31:51 +0000 (UTC) In-reply-to: <83wp57vmk6.fsf@gnu.org> (message from Eli Zaretskii on Sat, 09 Sep 2017 19:55:37 +0300) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:136716 Archived-At: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable > Date: Sat, 09 Sep 2017 19:55:37 +0300 > From: Eli Zaretskii > CC: 28350@debbugs.gnu.org > > > > > +See Info node `(elisp)Display Property' for the use of these > > > > +display specifications." > > > > + (ignore-errors > > > > + (or (stringp prop) > > > ^^^^^^^^^^^^ > > > What about an image spec (including a slice spec)? > >=20 > > Okay, I see that image specs can be safe. But are they all safe? >=20 > I think they are. Does anyone know different? I read over the documentation some more and they do look alright. > > And I don't understand how a slice spec is used together with an image > > spec. Is the slice spec used inside of IMAGE-PROPS, i.e. as you might > > gather from the manual: > >=20 > > =E2=80=98(image . IMAGE-PROPS)=E2=80=99 > > This kind of display specification is an image descriptor (*note > > Images). When used as a display specification, it means to > > display the image instead of the text that has the display > > specification. > >=20 > > =E2=80=98(slice X Y WIDTH HEIGHT)=E2=80=99 > > This specification together with =E2=80=98image=E2=80=99 specifies= a =E2=80=9Cslice=E2=80=9D (a > > partial area) of the image to display.=20 > >=20 > > ? >=20 > AFAIU, like this: >=20 > ((slice X Y WIDTH HEIGHT) (image . IMAGE-PROPS)) >=20 > You can see examples of this in image.el and image-mode.el. Thanks. I forgot that the display property can be set to a list or vector of display specifications. I've updated the patch to reflect this: + (and (seqp prop) (seq-every-p 'enriched-display-prop-safe-p prop))= ))) and I've added slice/image specifications. > > At this point it seems that unsafe display specs are more the > > exception than the rule, so it might make sense to define the > > `enriched-display-prop-safe-p' function by excluding the unsafe > > specifications instead of including the safe ones. What do you > > think? >=20 > I'm not sure. The display spec can be complex, so to make sure none > of these exceptions sneak through, you will have to recursively unpack > the spec data structure and examine each of the elements, which smells > too similar to emulating 'eval'. No? Thank you. I've kept the current approach. Please see again the attached patch. Also, should the left-fringe/right-fringe display specifications be considered safe? They seem innocuous. --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0001-Prevent-code-execution-by-text-enriched-files-Bug-28.patch >From baf533eeddc185a0e65c641022f7be2be2cbcb09 Mon Sep 17 00:00:00 2001 From: "Charles A. Roelli" 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-allow-unsafe-display-props): New customizable option. (enriched-display-prop-safe-p): New function. (enriched-decode-display-prop): Use the new function to prevent unsafe display properties from being applied. --- lisp/textmodes/enriched.el | 84 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 83 insertions(+), 1 deletion(-) diff --git a/lisp/textmodes/enriched.el b/lisp/textmodes/enriched.el index 7ace2a5..74a1229 100644 --- a/lisp/textmodes/enriched.el +++ b/lisp/textmodes/enriched.el @@ -147,6 +147,20 @@ enriched-mode-hook :type 'hook :group 'enriched) +(defcustom enriched-allow-unsafe-display-props nil + "Variable determining whether to decode arbitrary display properties. + +Enriched mode recognizes display properties of text stored using +an extension command to the text/enriched format, \"x-display\". +These properties must, by default, satisfy +`enriched-display-prop-safe-p' (q.v.), otherwise they are not +applied. Customize this option to t to turn off this safety +feature. Note, however, that applying unsafe display properties +can execute arbitrary Lisp code." + :risky t + :type 'boolean + :group 'enriched) + (defvar enriched-old-bindings nil "Store old variable values that we change when entering mode. The value is a list of \(VAR VALUE VAR VALUE...).") @@ -503,6 +517,74 @@ enriched-decode-display-prop (error nil))))) (unless prop (message "Warning: invalid parameter %s" param)) - (list start end 'display prop))) + (if (enriched-display-prop-safe-p prop) + (list start end 'display prop) + (message "Warning: unsafe 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. + +This function always returns t when +`enriched-allow-unsafe-display-props' is set to t. + +A safe display property is either: + + - a string, + + - an image display specification, (image . image-props), where + IMAGE-PROPS is a property list, + + - a slice display specification, (slice x y width height), + where X and Y are integers, and WIDTH and HEIGHT are either + integers or floats, + + - a space display specification, (space . props), where PROPS + is a property list, + + - 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 or an + image display specification as above, + + - a height display specification, (height spec), where SPEC is + of the form (+ n), (- n) or n, and N is an integer or a + float, + + - a raise display specification, (raise factor), where + FACTOR is an integer or a float, + + - or a list/vector containing safe display specifications, as + above. + +See Info node `(elisp)Display Property' for the use of these +display specifications." + (ignore-errors + (or enriched-allow-unsafe-display-props + (stringp prop) + (and (consp prop) (eq (car prop) 'image)) + (and (consp prop) + (eq (car prop) 'slice) + (integerp (elt prop 1)) ; x + (integerp (elt prop 2)) ; y + (numberp (elt prop 3)) ; width + (numberp (elt prop 4))) ; height + (and (consp prop) (eq (car prop) 'space)) + (and (eq (car prop) 'space-width) (numberp (cadr prop))) + (and (consp (car prop)) + (eq (caar prop) 'margin) + (or (eq (cadar prop) 'right-margin) + (eq (cadar prop) 'left-margin)) + (enriched-display-prop-safe-p (cadr prop))) + (and (eq (car prop) 'height) + (or (numberp (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) + (numberp (cadr prop))) + (and (seqp prop) (seq-every-p 'enriched-display-prop-safe-p prop))))) ;;; enriched.el ends here -- 2.9.4 --=-=-=--