CVE-2023-28617: [PATCH] Fix ob-latex.el command injection vulnerability.

* [PATCH] Fix ob-latex.el command injection vulnerability. @ 2023-02-18 10:08 lux 2023-02-18 11:15 ` Max Nikulin 0 siblings, 1 reply; 20+ messages in thread From: lux @ 2023-02-18 10:08 UTC (permalink / raw) To: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 1737 bytes --]

Test environment:

• Emacs 29.0.60
• Orgmode 9.6.1
• TeX Live 2020

Preconditions:

(org-babel-do-load-languages 'org-babel-load-languages '((latex . t)))

The vulnerability occurs in the file ob-latex.el, in the `org-babel- execute:latex’ function, if then file’s extension is .svg, using `shell-command’ function to call the `mv’ shell command:

((string= “svg” extension) … (let ((tmp-pdf (org-babel-latex-tex-to-pdf tex-file))) (let* (… (img-out (org-compile-file tmp-pdf (list org-babel-latex-pdf-svg-process) extension err-msg log-buf))) (shell-command (format “mv %s %s” img-out out-file)))))

But the parameter `img-out’ and parameter `out-file’ are not escape. So, if file name or directory name contains shell characters and will be executed.

Example for the vul_test.org file:

#+name: vul_test #+header: :file test;uname -a;.svg #+begin_src latex \LaTeX #+end_src

Using Emacs open it, and press ‘C-c C-e l p’ export to a pdf file, or point to begin_src block and press ‘C-c C-c’ to execute block.

In the ‘*Message*’ buffer, you can see the ‘uname -a’ command output:

Executing Latex code block (vul_test)… Processing LaTeX file /tmp/babel-UCtwdU/latex-zWDsHS.tex… PDF file produced.

,** (org.inkscape.Inkscape:145910): WARNING **: 17:27:24.285: Fonts dir ‘/usr/share/inkscape/fonts’ does not exist and will be ignored. Linux lx-debian 5.10.0-21-amd64 #1 SMP Debian 5.10.162-1 (2023-01-21) x86_64 GNU/Linux <---- This is ‘uname -a’ output zsh:1: command not found: .svg Code block produced no output (took 1.1s).

This patch fixed it.

[-- Attachment #2: 0001-lisp-ob-latex.el-org-babel-execute-latex-Fix-command.patch --] [-- Type: text/x-patch, Size: 1021 bytes --]

From 422ffedc32c31fef39d943612d7e738cf4ad5e23 Mon Sep 17 00:00:00 2001 From: Xi Lu [email protected] Date: Sat, 18 Feb 2023 18:03:28 +0800 Subject: [PATCH] * lisp/ob-latex.el (org-babel-execute:latex): Fix command injection vulnerability.

---

lisp/ob-latex.el | 2 ± 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/ob-latex.el b/lisp/ob-latex.el index 428907a27…c32e7ea4c 100644 — a/lisp/ob-latex.el +++ b/lisp/ob-latex.el @@ -180,7 +180,7 @@ This function is called by `org-babel-execute-src-block’." tmp-pdf (list org-babel-latex-pdf-svg-process) extension err-msg log-buf))) - (shell-command (format “mv %s %s” img-out out-file))))) + (shell-command (format “mv %s %s” (shell-quote-argument img-out) (shell-quote-argument out-file)))))) ((string-suffix-p “.tikz” out-file) (when (file-exists-p out-file) (delete-file out-file)) (with-temp-file out-file – 2.30.2

^ permalink raw reply related [flat|nested] 20+ messages in thread

* Re: [PATCH] Fix ob-latex.el command injection vulnerability. 2023-02-18 10:08 [PATCH] Fix ob-latex.el command injection vulnerability lux @ 2023-02-18 11:15 ` Max Nikulin 2023-02-18 11:28 ` lux 0 siblings, 1 reply; 20+ messages in thread From: Max Nikulin @ 2023-02-18 11:15 UTC (permalink / raw) To: lux, emacs-orgmode

On 18/02/2023 17:08, lux wrote: > - (shell-command (format “mv %s %s” img-out out-file)))))

•          (shell-command (format "mv %s %s" (shell-quote-argument img-out) (shell-quote-argument out-file))))))
  

Thank you for the patch. Certainly it is an improvement.

Is there any reason why `rename-file’ should be avoided here? I just have discovered this function, so I am unaware of possible pitfalls.

(info "(elisp) Changing-Files") https://www.gnu.org/software/emacs/manual/html_node/elisp/Changing-Files.html#index-rename_002dfile

^ permalink raw reply [flat|nested] 20+ messages in thread

* Re: [PATCH] Fix ob-latex.el command injection vulnerability. 2023-02-18 11:15 ` Max Nikulin @ 2023-02-18 11:28 ` lux 2023-02-18 11:43 ` Ihor Radchenko 0 siblings, 1 reply; 20+ messages in thread From: lux @ 2023-02-18 11:28 UTC (permalink / raw) To: Max Nikulin, emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 856 bytes --]

On Sat, 2023-02-18 at 18:15 +0700, Max Nikulin wrote: > On 18/02/2023 17:08, lux wrote:

•          (shell-command (format "mv %s %s" img-out out-
  

file)))))

•          (shell-command (format "mv %s %s" (shell-quote-
  

argument img-out) (shell-quote-argument out-file))))))

Thank you for the patch. Certainly it is an improvement.

Is there any reason why `rename-file’ should be avoided here? I just have discovered this function, so I am unaware of possible pitfalls.

(info "(elisp) Changing-Files") https://www.gnu.org/software/emacs/manual/html_node/elisp/Changing-Files.html#index-rename_002dfile I think using `rename-file’ is good idea. We should use the Emacs built-in functions as much as possible instead of external shell commands, becueas these more security.

[-- Attachment #2: 0001-lisp-ob-latex.el-org-babel-execute-latex-Fix-command.patch --] [-- Type: text/x-patch, Size: 953 bytes --]

From adc0c558b1b091bb4bef77901633f31344b7391a Mon Sep 17 00:00:00 2001 From: Xi Lu [email protected] Date: Sat, 18 Feb 2023 18:03:28 +0800 Subject: [PATCH] * lisp/ob-latex.el (org-babel-execute:latex): Fix command injection vulnerability.

---

lisp/ob-latex.el | 2 ± 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/ob-latex.el b/lisp/ob-latex.el index 428907a27…0d0a37a02 100644 — a/lisp/ob-latex.el +++ b/lisp/ob-latex.el @@ -180,7 +180,7 @@ This function is called by `org-babel-execute-src-block’." tmp-pdf (list org-babel-latex-pdf-svg-process) extension err-msg log-buf))) - (shell-command (format “mv %s %s” img-out out-file))))) + (rename-file img-out out-file)))) ((string-suffix-p “.tikz” out-file) (when (file-exists-p out-file) (delete-file out-file)) (with-temp-file out-file – 2.30.2

^ permalink raw reply related [flat|nested] 20+ messages in thread

* Re: [PATCH] Fix ob-latex.el command injection vulnerability. 2023-02-18 11:28 ` lux @ 2023-02-18 11:43 ` Ihor Radchenko 2023-02-19 2:31 ` lux 2023-03-06 3:17 ` lux 0 siblings, 2 replies; 20+ messages in thread From: Ihor Radchenko @ 2023-02-18 11:43 UTC (permalink / raw) To: lux; +Cc: Max Nikulin, emacs-orgmode

lux [email protected] writes:

> - (shell-command (format “mv %s %s” img-out out-file)))))

•          (rename-file img-out out-file))))
  

I think should be (rename-file img-out out-file t)

– Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at https://orgmode.org/\. Support Org development at https://liberapay.com/org-mode\, or support my work at https://liberapay.com/yantar92\

^ permalink raw reply [flat|nested] 20+ messages in thread

* Re: [PATCH] Fix ob-latex.el command injection vulnerability. 2023-02-18 11:43 ` Ihor Radchenko @ 2023-02-19 2:31 ` lux 2023-03-06 3:17 ` lux 1 sibling, 0 replies; 20+ messages in thread From: lux @ 2023-02-19 2:31 UTC (permalink / raw) To: Ihor Radchenko; +Cc: Max Nikulin, emacs-orgmode

On Sat, 2023-02-18 at 11:43 +0000, Ihor Radchenko wrote: > lux [email protected] writes:

•          (shell-command (format "mv %s %s" img-out out-
  

file)))))

•          (rename-file img-out out-file))))
  

I think should be (rename-file img-out out-file t)

Yes, my pachted changed it, thank you.

^ permalink raw reply [flat|nested] 20+ messages in thread

* Re: [PATCH] Fix ob-latex.el command injection vulnerability. 2023-02-18 11:43 ` Ihor Radchenko 2023-02-19 2:31 ` lux @ 2023-03-06 3:17 ` lux 2023-03-07 12:35 ` Ihor Radchenko 2023-03-07 15:31 ` Max Nikulin 1 sibling, 2 replies; 20+ messages in thread From: lux @ 2023-03-06 3:17 UTC (permalink / raw) To: Ihor Radchenko; +Cc: Max Nikulin, emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 342 bytes --]

On Sat, 2023-02-18 at 11:43 +0000, Ihor Radchenko wrote: > lux [email protected] writes:

•          (shell-command (format "mv %s %s" img-out out-
  

file)))))

•          (rename-file img-out out-file))))
  

I think should be (rename-file img-out out-file t)

Fixed, thank you.

[-- Attachment #2: 0001-lisp-ob-latex.el-org-babel-execute-latex-Fix-command.patch --] [-- Type: text/x-patch, Size: 955 bytes --]

From adc0c558b1b091bb4bef77901633f31344b7391a Mon Sep 17 00:00:00 2001 From: Xi Lu [email protected] Date: Sat, 18 Feb 2023 18:03:28 +0800 Subject: [PATCH] * lisp/ob-latex.el (org-babel-execute:latex): Fix command injection vulnerability.

---

lisp/ob-latex.el | 2 ± 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/ob-latex.el b/lisp/ob-latex.el index 428907a27…0d0a37a02 100644 — a/lisp/ob-latex.el +++ b/lisp/ob-latex.el @@ -180,7 +180,7 @@ This function is called by `org-babel-execute-src-block’." tmp-pdf (list org-babel-latex-pdf-svg-process) extension err-msg log-buf))) - (shell-command (format “mv %s %s” img-out out-file))))) + (rename-file img-out out-file t)))) ((string-suffix-p “.tikz” out-file) (when (file-exists-p out-file) (delete-file out-file)) (with-temp-file out-file – 2.30.2

^ permalink raw reply related [flat|nested] 20+ messages in thread

* Re: [PATCH] Fix ob-latex.el command injection vulnerability. 2023-03-06 3:17 ` lux @ 2023-03-07 12:35 ` Ihor Radchenko 2023-03-07 13:20 ` lux 2023-03-07 15:31 ` Max Nikulin 1 sibling, 1 reply; 20+ messages in thread From: Ihor Radchenko @ 2023-03-07 12:35 UTC (permalink / raw) To: lux; +Cc: Max Nikulin, emacs-orgmode

lux [email protected] writes:

>> I think should be (rename-file img-out out-file t)

Fixed, thank you. Applied, onto bugfix. I amended the commit message adding a link to this thread and a TINYCHANGE cookie (you don’t seem to have FSF copyright assignment).

https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=8f8ec2ccf

Thanks for your contribution!

You are now listed as an Org contributor. https://git.sr.ht/~bzg/worg/commit/f59fbd00

– Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at https://orgmode.org/\. Support Org development at https://liberapay.com/org-mode\, or support my work at https://liberapay.com/yantar92\

^ permalink raw reply [flat|nested] 20+ messages in thread

* Re: [PATCH] Fix ob-latex.el command injection vulnerability. 2023-03-07 12:35 ` Ihor Radchenko @ 2023-03-07 13:20 ` lux 2023-03-07 13:52 ` Ihor Radchenko 0 siblings, 1 reply; 20+ messages in thread From: lux @ 2023-03-07 13:20 UTC (permalink / raw) To: Ihor Radchenko; +Cc: Max Nikulin, emacs-orgmode

On Tue, 2023-03-07 at 12:35 +0000, Ihor Radchenko wrote: > lux [email protected] writes:

> (you don’t seem to have FSF copyright

assignment).

Thank you :-)

I have already signed a copyright assignment with Emacs, do I need to sign again with Org Mode?

^ permalink raw reply [flat|nested] 20+ messages in thread

* Re: [PATCH] Fix ob-latex.el command injection vulnerability. 2023-03-07 13:20 ` lux @ 2023-03-07 13:52 ` Ihor Radchenko 2023-03-07 15:06 ` Bastien Guerry 0 siblings, 1 reply; 20+ messages in thread From: Ihor Radchenko @ 2023-03-07 13:52 UTC (permalink / raw) To: lux, Bastien; +Cc: Max Nikulin, emacs-orgmode

lux [email protected] writes:

>> (you don’t seem to have FSF copyright

assignment).

Thank you :-)

I have already signed a copyright assignment with Emacs, do I need to sign again with Org Mode? No, you don’t. Emacs assignment is sufficient. Bastien, may you check the FSF records for Xi Lu?

– Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at https://orgmode.org/\. Support Org development at https://liberapay.com/org-mode\, or support my work at https://liberapay.com/yantar92\

^ permalink raw reply [flat|nested] 20+ messages in thread

* Re: [PATCH] Fix ob-latex.el command injection vulnerability. 2023-03-07 13:52 ` Ihor Radchenko @ 2023-03-07 15:06 ` Bastien Guerry 2023-03-07 15:10 ` Ihor Radchenko 0 siblings, 1 reply; 20+ messages in thread From: Bastien Guerry @ 2023-03-07 15:06 UTC (permalink / raw) To: Ihor Radchenko; +Cc: lux, Max Nikulin, emacs-orgmode

Hi,

Ihor Radchenko [email protected] writes:

> Bastien, may you check the FSF records for Xi Lu? I confirm Xi Lu FSF assignment is in order.

Thanks for contributing!

– Bastien Guerry

^ permalink raw reply [flat|nested] 20+ messages in thread

* Re: [PATCH] Fix ob-latex.el command injection vulnerability. 2023-03-07 15:06 ` Bastien Guerry @ 2023-03-07 15:10 ` Ihor Radchenko 0 siblings, 0 replies; 20+ messages in thread From: Ihor Radchenko @ 2023-03-07 15:10 UTC (permalink / raw) To: Bastien Guerry; +Cc: lux, Max Nikulin, emacs-orgmode

Bastien Guerry [email protected] writes:

> Hi,

Ihor Radchenko [email protected] writes:

Bastien, may you check the FSF records for Xi Lu?

I confirm Xi Lu FSF assignment is in order. Updated on Worg. https://git.sr.ht/~bzg/worg/commit/e94905a7

– Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at https://orgmode.org/\. Support Org development at https://liberapay.com/org-mode\, or support my work at https://liberapay.com/yantar92\

^ permalink raw reply [flat|nested] 20+ messages in thread

* Re: [PATCH] Fix ob-latex.el command injection vulnerability. 2023-03-06 3:17 ` lux 2023-03-07 12:35 ` Ihor Radchenko @ 2023-03-07 15:31 ` Max Nikulin 2023-03-08 2:28 ` lux 2023-03-08 15:42 ` lux 1 sibling, 2 replies; 20+ messages in thread From: Max Nikulin @ 2023-03-07 15:31 UTC (permalink / raw) To: emacs-orgmode; +Cc: lux

On 06/03/2023 10:17, lux wrote: > On Sat, 2023-02-18 at 11:43 +0000, Ihor Radchenko wrote:

I think should be (rename-file img-out out-file t)

Fixed, thank you. There are a couple more mv shell commands in ob-latex.el. It would be nice to fix them as well. Sorry, I have not checked it earlier. Are you still interested in this topic? I hope, you already have examples that can be used to quickly test if modified code works as expected.

^ permalink raw reply [flat|nested] 20+ messages in thread

* Re: [PATCH] Fix ob-latex.el command injection vulnerability. 2023-03-07 15:31 ` Max Nikulin @ 2023-03-08 2:28 ` lux 2023-03-08 15:42 ` lux 1 sibling, 0 replies; 20+ messages in thread From: lux @ 2023-03-08 2:28 UTC (permalink / raw) To: Max Nikulin, emacs-orgmode

On Tue, 2023-03-07 at 22:31 +0700, Max Nikulin wrote: > There are a couple more mv shell commands in ob-latex.el. It would be

nice to fix them as well. Sorry, I have not checked it earlier. Are you still interested in this topic? I hope, you already have examples that can be used to quickly test if modified code works as expected. No problem, I recently fix and test.

^ permalink raw reply [flat|nested] 20+ messages in thread

* Re: [PATCH] Fix ob-latex.el command injection vulnerability. 2023-03-07 15:31 ` Max Nikulin 2023-03-08 2:28 ` lux @ 2023-03-08 15:42 ` lux 2023-03-09 12:22 ` Ihor Radchenko 1 sibling, 1 reply; 20+ messages in thread From: lux @ 2023-03-08 15:42 UTC (permalink / raw) To: Max Nikulin, emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 873 bytes --]

On Tue, 2023-03-07 at 22:31 +0700, Max Nikulin wrote: > On 06/03/2023 10:17, lux wrote:

On Sat, 2023-02-18 at 11:43 +0000, Ihor Radchenko wrote:

I think should be (rename-file img-out out-file t)

Fixed, thank you.

There are a couple more mv shell commands in ob-latex.el. It would be nice to fix them as well. Sorry, I have not checked it earlier. Are you still interested in this topic? I hope, you already have examples that can be used to quickly test if modified code works as expected. Hi, this is a new patch, let me briefly explain this patch:

1. Replaced the `(shell-command “mv BAR NEWBAR”)' with `rename-file’.

2. `org-babel-latex-convert-pdf’ is not safe, simple test:

   (org-babel-latex-convert-pdf “;id;.tex” “;uname;.pdf” “” “”)

So, add `shell-quote-argument’ to each external parameter.

[-- Attachment #2: 0001-lisp-ob-latex.el-Fix-command-injection-vulnerability.patch --] [-- Type: text/x-patch, Size: 2298 bytes --]

From 62f9d32decdd078633e51ea9fa30fdb000b6de51 Mon Sep 17 00:00:00 2001 From: Xi Lu [email protected] Date: Wed, 8 Mar 2023 23:28:32 +0800 Subject: [PATCH] * lisp/ob-latex.el: Fix command injection vulnerability

(org-babel-execute:latex): Fix command injection vulnerability (org-babel-latex-convert-pdf): Add `shell-quote-argument’

---

lisp/ob-latex.el | 19 ++++++++±--------- 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/lisp/ob-latex.el b/lisp/ob-latex.el index a2c24b3d9…2315a8b7c 100644 — a/lisp/ob-latex.el +++ b/lisp/ob-latex.el @@ -218,17 +218,14 @@ This function is called by `org-babel-execute-src-block’." (if (string-suffix-p “.svg” out-file) (progn (shell-command “pwd”) - (shell-command (format “mv %s %s”

•                (concat (file-name-sans-extension tex-file) "-1.svg")
  

•                out-file)))
  

+ (rename-file (concat (file-name-sans-extension tex-file) "-1.svg")

•                           out-file t))
      (error "SVG file produced but HTML file requested")))
   ((file-exists-p (concat (file-name-sans-extension tex-file) ".html"))
    (if (string-suffix-p ".html" out-file)
  

- (shell-command “mv %s %s”

•              (concat (file-name-sans-extension tex-file)
  

•                  ".html")
  

•              out-file)
  

•     (error "HTML file produced but SVG file requested")))))
  

+ (rename-file (concat (file-name-sans-extension tex-file) “.html”)

•                         out-file t)
  

•          (error "HTML file produced but SVG file requested")))))
  

  ((or (string= “pdf” extension) imagemagick) (with-temp-file tex-file (require 'ox-latex) @@ -277,8 +274,10 @@ This function is called by `org-babel-execute-src-block’."

(defun org-babel-latex-convert-pdf (pdffile out-file im-in-options im-out-options) “Generate a file from a pdf file using imagemagick.” - (let ((cmd (concat "convert " im-in-options " " pdffile " "

•        im-out-options " " out-file)))
  

+ (let ((cmd (concat "convert " (shell-quote-argument im-in-options) " "

•                 (shell-quote-argument pdffile) " "
  

•        (shell-quote-argument im-out-options) " "
  

•                 (shell-quote-argument out-file))))
  

  (message “Converting pdffile file %s…” cmd) (shell-command cmd)))

– 2.39.2

^ permalink raw reply related [flat|nested] 20+ messages in thread

* Re: [PATCH] Fix ob-latex.el command injection vulnerability. 2023-03-08 15:42 ` lux @ 2023-03-09 12:22 ` Ihor Radchenko 2023-03-09 16:29 ` Max Nikulin 2023-03-11 5:12 ` lux 0 siblings, 2 replies; 20+ messages in thread From: Ihor Radchenko @ 2023-03-09 12:22 UTC (permalink / raw) To: lux; +Cc: Max Nikulin, emacs-orgmode

lux [email protected] writes:

> Hi, this is a new patch, let me briefly explain this patch: Thanks!

> 2. `org-babel-latex-convert-pdf’ is not safe, simple test:

(org-babel-latex-convert-pdf “;id;.tex” “;uname;.pdf” “” “”)

So, add `shell-quote-argument’ to each external parameter. I am not sure if blindly adding `shell-quote-argument’ is safe here.

> (defun org-babel-latex-convert-pdf (pdffile out-file im-in-options im-out-options)

“Generate a file from a pdf file using imagemagick.”

• (let ((cmd (concat "convert " im-in-options " " pdffile " "

•      im-out-options " " out-file)))
  

• (let ((cmd (concat "convert " (shell-quote-argument im-in-options) " "

•                 (shell-quote-argument pdffile) " "
  

•      (shell-quote-argument im-out-options) " "
  

•                 (shell-quote-argument out-file))))
  

  (message “Converting pdffile file %s…” cmd) (shell-command cmd))) im-in-options and im-out-options, according to https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-LaTeX.html, are options passed to ImageMagick.

However, for example, (shell-quote-argument "-enhance -strip") will return "-enhance\\ -strip", which is not what we want.

Similar problem with other instances of `shell-command’ in Org where header args supply command line arguments. Like in :cmdline.

– Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at https://orgmode.org/\. Support Org development at https://liberapay.com/org-mode\, or support my work at https://liberapay.com/yantar92\

^ permalink raw reply [flat|nested] 20+ messages in thread

* Re: [PATCH] Fix ob-latex.el command injection vulnerability. 2023-03-09 12:22 ` Ihor Radchenko @ 2023-03-09 16:29 ` Max Nikulin 2023-03-11 5:12 ` lux 1 sibling, 0 replies; 20+ messages in thread From: Max Nikulin @ 2023-03-09 16:29 UTC (permalink / raw) To: emacs-orgmode

On 09/03/2023 19:22, Ihor Radchenko wrote: > lux writes:

Hi, this is a new patch, let me briefly explain this patch: Thank you for scratching my itch related to unsafe shell commands in Org Mode.

>> 2. `org-babel-latex-convert-pdf’ is not safe, simple test: … > I am not sure if blindly adding `shell-quote-argument’ is safe here. I believe, first hunk still can be committed.

>> (shell-command cmd)))

im-in-options and im-out-options, according to https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-LaTeX.html, are options passed to ImageMagick. ImageMagick is disaster per se.

Ideally `call-process’ or `process-file’ should be here instead of `shell-command’ making `shell-quote-argument’ unnecessary. Sorry, it is not clear for me if remote files (e.g. /ssh:…) are supported here. Unfortunately options as a string, not as a list, means compatibility issue. `split-string-and-unquote’ may cause new bugs.

I have not evaluated it yet, but from discussions on this list I have an impression that some LaTeX packages need to run external commands. I am unsure to which degree it is safe or it may be easily exploited.

^ permalink raw reply [flat|nested] 20+ messages in thread

* Re: [PATCH] Fix ob-latex.el command injection vulnerability. 2023-03-09 12:22 ` Ihor Radchenko 2023-03-09 16:29 ` Max Nikulin @ 2023-03-11 5:12 ` lux 2023-03-11 10:47 ` Ihor Radchenko 1 sibling, 1 reply; 20+ messages in thread From: lux @ 2023-03-11 5:12 UTC (permalink / raw) To: Ihor Radchenko; +Cc: Max Nikulin, emacs-orgmode

On Thu, 2023-03-09 at 12:22 +0000, Ihor Radchenko wrote: >

im-in-options and im-out-options, according to https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-LaTeX.html , are options passed to ImageMagick.

However, for example, (shell-quote-argument "-enhance -strip") will return "-enhance\\ -strip", which is not what we want.

Similar problem with other instances of `shell-command’ in Org where header args supply command line arguments. Like in :cmdline.

I think there is only a need to deal with the problem of `\\ ', for example:

(string-replace "\\ " " " (shell-quote-argument im-in-options))

Any better suggestions? Thanks.

^ permalink raw reply [flat|nested] 20+ messages in thread

* Re: [PATCH] Fix ob-latex.el command injection vulnerability. 2023-03-11 5:12 ` lux @ 2023-03-11 10:47 ` Ihor Radchenko 2023-03-11 10:57 ` lux 0 siblings, 1 reply; 20+ messages in thread From: Ihor Radchenko @ 2023-03-11 10:47 UTC (permalink / raw) To: lux; +Cc: Max Nikulin, emacs-orgmode

lux [email protected] writes:

>> However, for example, (shell-quote-argument "-enhance -strip") will

return "-enhance\\ -strip", which is not what we want.

Similar problem with other instances of `shell-command’ in Org where header args supply command line arguments. Like in :cmdline.

I think there is only a need to deal with the problem of `\\ ', for example:

(string-replace "\\ " " " (shell-quote-argument im-in-options)) No. Quoting will also affect ‘"’ and other '\’. We must not try to be smart here. It will miss edge cases and be fragile in general.

> Any better suggestions? Thanks. I am afraid that we cannot make things universally safe here without breaking changes. The best way will be treating :cmd and similar header args as unsafe and include them into the planned safety prompt system we discussed in https://orgmode.org/list/87edsd5o89.fsf@localhost

– Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at https://orgmode.org/\. Support Org development at https://liberapay.com/org-mode\, or support my work at https://liberapay.com/yantar92\

^ permalink raw reply [flat|nested] 20+ messages in thread

* Re: [PATCH] Fix ob-latex.el command injection vulnerability. 2023-03-11 10:47 ` Ihor Radchenko @ 2023-03-11 10:57 ` lux 2023-03-12 11:28 ` Ihor Radchenko 0 siblings, 1 reply; 20+ messages in thread From: lux @ 2023-03-11 10:57 UTC (permalink / raw) To: Ihor Radchenko; +Cc: Max Nikulin, emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 428 bytes --]

On Sat, 2023-03-11 at 10:47 +0000, Ihor Radchenko wrote: >

I am afraid that we cannot make things universally safe here without breaking changes. The best way will be treating :cmd and similar header args as unsafe and include them into the planned safety prompt system we discussed in https://orgmode.org/list/87edsd5o89.fsf@localhost

Ok, I’ll undo this part of the changes first, and repost patch.

[-- Attachment #2: 0001-lisp-ob-latex.el-Fix-command-injection-vulnerability.patch --] [-- Type: text/x-patch, Size: 1623 bytes --]

From b48784a16c5806694498f072ffdd98e5a3c144b5 Mon Sep 17 00:00:00 2001 From: Xi Lu [email protected] Date: Sat, 11 Mar 2023 18:53:37 +0800 Subject: [PATCH] * lisp/ob-latex.el: Fix command injection vulnerability

(org-babel-execute:latex): Replaced the `(shell-command “mv BAR NEWBAR”)' with `rename-file’.

---

lisp/ob-latex.el | 13 ++++±------- 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/lisp/ob-latex.el b/lisp/ob-latex.el index a2c24b3d9…ce39628d6 100644 — a/lisp/ob-latex.el +++ b/lisp/ob-latex.el @@ -218,17 +218,14 @@ This function is called by `org-babel-execute-src-block’." (if (string-suffix-p “.svg” out-file) (progn (shell-command “pwd”) - (shell-command (format “mv %s %s”

•                (concat (file-name-sans-extension tex-file) "-1.svg")
  

•                out-file)))
  

+ (rename-file (concat (file-name-sans-extension tex-file) "-1.svg")

•                           out-file t))
      (error "SVG file produced but HTML file requested")))
   ((file-exists-p (concat (file-name-sans-extension tex-file) ".html"))
    (if (string-suffix-p ".html" out-file)
  

- (shell-command “mv %s %s”

•              (concat (file-name-sans-extension tex-file)
  

•                  ".html")
  

•              out-file)
  

•     (error "HTML file produced but SVG file requested")))))
  

+ (rename-file (concat (file-name-sans-extension tex-file) “.html”)

•                         out-file t)
  

•          (error "HTML file produced but SVG file requested")))))
  

  ((or (string= “pdf” extension) imagemagick) (with-temp-file tex-file (require 'ox-latex) – 2.39.2

^ permalink raw reply related [flat|nested] 20+ messages in thread

* Re: [PATCH] Fix ob-latex.el command injection vulnerability. 2023-03-11 10:57 ` lux @ 2023-03-12 11:28 ` Ihor Radchenko 0 siblings, 0 replies; 20+ messages in thread From: Ihor Radchenko @ 2023-03-12 11:28 UTC (permalink / raw) To: lux; +Cc: Max Nikulin, emacs-orgmode

lux [email protected] writes:

> Ok, I’ll undo this part of the changes first, and repost patch.

From b48784a16c5806694498f072ffdd98e5a3c144b5 Mon Sep 17 00:00:00 2001 From: Xi Lu [email protected] Date: Sat, 11 Mar 2023 18:53:37 +0800 Subject: [PATCH] * lisp/ob-latex.el: Fix command injection vulnerability Thanks! Applied, onto bugfix. https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=a8006ea58

– Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at https://orgmode.org/\. Support Org development at https://liberapay.com/org-mode\, or support my work at https://liberapay.com/yantar92\

^ permalink raw reply [flat|nested] 20+ messages in thread

end of thread, other threads:[~2023-03-12 11:27 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) – links below jump to the message on this page – 2023-02-18 10:08 [PATCH] Fix ob-latex.el command injection vulnerability lux 2023-02-18 11:15 ` Max Nikulin 2023-02-18 11:28 ` lux 2023-02-18 11:43 ` Ihor Radchenko 2023-02-19 2:31 ` lux 2023-03-06 3:17 ` lux 2023-03-07 12:35 ` Ihor Radchenko 2023-03-07 13:20 ` lux 2023-03-07 13:52 ` Ihor Radchenko 2023-03-07 15:06 ` Bastien Guerry 2023-03-07 15:10 ` Ihor Radchenko 2023-03-07 15:31 ` Max Nikulin 2023-03-08 2:28 ` lux 2023-03-08 15:42 ` lux 2023-03-09 12:22 ` Ihor Radchenko 2023-03-09 16:29 ` Max Nikulin 2023-03-11 5:12 ` lux 2023-03-11 10:47 ` Ihor Radchenko 2023-03-11 10:57 ` lux 2023-03-12 11:28 ` Ihor Radchenko

Code repositories for project(s) associated with this public inbox

https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).