[PATCH] Fix ob-latex.el command injection vulnerability.

[PATCH] Fix ob-latex.el command injection vulnerability.

[lux]

[-- 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 <lx@shellcodes.org>
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

Re: [PATCH] Fix ob-latex.el command injection vulnerability.

[Max Nikulin]

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

Re: [PATCH] Fix ob-latex.el command injection vulnerability.

[lux]

[-- 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 <lx@shellcodes.org>
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

Re: [PATCH] Fix ob-latex.el command injection vulnerability.

[Ihor Radchenko]

lux <lx@shellcodes.org> 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>

Re: [PATCH] Fix ob-latex.el command injection vulnerability.

[lux]

On Sat, 2023-02-18 at 11:43 +0000, Ihor Radchenko wrote:
> lux <lx@shellcodes.org> 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.

Re: [PATCH] Fix ob-latex.el command injection vulnerability.

[lux]

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

On Sat, 2023-02-18 at 11:43 +0000, Ihor Radchenko wrote:
> lux <lx@shellcodes.org> 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 <lx@shellcodes.org>
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

Re: [PATCH] Fix ob-latex.el command injection vulnerability.

[Ihor Radchenko]

lux <lx@shellcodes.org> 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>

Re: [PATCH] Fix ob-latex.el command injection vulnerability.

[lux]

On Tue, 2023-03-07 at 12:35 +0000, Ihor Radchenko wrote:
> lux <lx@shellcodes.org> 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?

Re: [PATCH] Fix ob-latex.el command injection vulnerability.

[Ihor Radchenko]

lux <lx@shellcodes.org> 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>

Re: [PATCH] Fix ob-latex.el command injection vulnerability.

[Bastien Guerry]

Hi,

Ihor Radchenko <yantar92@posteo.net> 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

Re: [PATCH] Fix ob-latex.el command injection vulnerability.

[Ihor Radchenko]

Bastien Guerry <bzg@gnu.org> writes:

> Hi,
>
> Ihor Radchenko <yantar92@posteo.net> 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>

Re: [PATCH] Fix ob-latex.el command injection vulnerability.

[Max Nikulin]

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.

Re: [PATCH] Fix ob-latex.el command injection vulnerability.

[lux]

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.

Re: [PATCH] Fix ob-latex.el command injection vulnerability.

[lux]

[-- 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 <lx@shellcodes.org>
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

Re: [PATCH] Fix ob-latex.el command injection vulnerability.

[Ihor Radchenko]

lux <lx@shellcodes.org> 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>

Re: [PATCH] Fix ob-latex.el command injection vulnerability.

[Max Nikulin]

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.

Re: [PATCH] Fix ob-latex.el command injection vulnerability.

[lux]

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.

Re: [PATCH] Fix ob-latex.el command injection vulnerability.

[Ihor Radchenko]

lux <lx@shellcodes.org> 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>

Re: [PATCH] Fix ob-latex.el command injection vulnerability.

[lux]

[-- 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 <lx@shellcodes.org>
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

Re: [PATCH] Fix ob-latex.el command injection vulnerability.

[Ihor Radchenko]

lux <lx@shellcodes.org> 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 <lx@shellcodes.org>
> 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>

Re: [PATCH] Fix ob-latex.el command injection vulnerability.

[Max Nikulin]

On 12/03/2023 18:28, Ihor Radchenko wrote:
> lux 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
>> Date: Sat, 11 Mar 2023 18:53:37 +0800
>> Subject: [PATCH] * lisp/ob-latex.el: Fix command injection vulnerability
> 
> Applied, onto bugfix.
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=a8006ea58

So the fix is included into org-mode-9.6.2.

I just have noticed that it is tracked as a CVE record:

https://www.cve.org/CVERecord?id=CVE-2023-28617
https://nvd.nist.gov/vuln/detail/CVE-2023-28617

CVE-2023-28617
org-babel-execute:latex in ob-latex.el in Org Mode through 9.6.1 for GNU 
Emacs allows attackers to execute arbitrary commands via a file name or 
directory name that contains shell metacharacters.

Base Score:  7.8 HIGH

Re: [PATCH] Fix ob-latex.el command injection vulnerability.

[Ihor Radchenko]

Max Nikulin <manikulin@gmail.com> writes:

> I just have noticed that it is tracked as a CVE record:
>
> https://www.cve.org/CVERecord?id=CVE-2023-28617
> https://nvd.nist.gov/vuln/detail/CVE-2023-28617

And we do not need to do anything about it, right?

-- 
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>

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

[Max Nikulin]

On 01/05/2023 18:18, Ihor Radchenko wrote:
> Max Nikulin writes:
> 
>> I just have noticed that it is tracked as a CVE record:
>>
>> https://www.cve.org/CVERecord?id=CVE-2023-28617
>> https://nvd.nist.gov/vuln/detail/CVE-2023-28617
> 
> And we do not need to do anything about it, right?

I posted the links as a reminder that shell commands should be avoided 
when possible (and it does not break TRAMP) and arguments should be 
escaped otherwise.

I suppose, the issue has been received too much attention already:

- https://security-tracker.debian.org/tracker/CVE-2023-28617
- https://ubuntu.com/security/notices/USN-6003-1
- https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2023-28617
etc.

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

[Ihor Radchenko]

Max Nikulin <manikulin@gmail.com> writes:

>> And we do not need to do anything about it, right?
>
> I posted the links as a reminder that shell commands should be avoided 
> when possible (and it does not break TRAMP) and arguments should be 
> escaped otherwise.

But this patch literally fixed the problem. What else should we do?

> I suppose, the issue has been received too much attention already:
>
> - https://security-tracker.debian.org/tracker/CVE-2023-28617
> - https://ubuntu.com/security/notices/USN-6003-1
> - https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2023-28617

These appear to be different issues.
If you think that any of them remains unaddressed, patches welcome.

-- 
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>

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

[Max Nikulin]

On 02/05/2023 18:21, Ihor Radchenko wrote:
> Max Nikulin writes:
>>
>> I posted the links as a reminder that shell commands should be avoided
>> when possible (and it does not break TRAMP) and arguments should be
>> escaped otherwise.
> 
> But this patch literally fixed the problem. What else should we do?

Do you really think that it was the last unsafe shell command in the Org 
code?

https://git.savannah.gnu.org/cgit/emacs/org-mode.git/tree/lisp/ob-ditaa.el#n101
and (shell-command pdf-cmd) below

https://git.savannah.gnu.org/cgit/emacs/org-mode.git/tree/lisp/ob-lilypond.el#n194

Of course, you may say that expanding shell constructs in :file header 
argument is a feature that allows more flexibility. Personally, I 
inspect Org files obtained from external resources in some dumb enough 
viewer before opening them in Emacs.

>> I suppose, the issue has been received too much attention already:
>>
>> - https://security-tracker.debian.org/tracker/CVE-2023-28617
>> - https://ubuntu.com/security/notices/USN-6003-1
>> - https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2023-28617
> 
> These appear to be different issues.

Linux distributions had to react to the CVE with formally high score and 
updated Emacs packages applying 2 tiny patches from this thread.

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

[Ihor Radchenko]

Max Nikulin <manikulin@gmail.com> writes:

>> But this patch literally fixed the problem. What else should we do?
>
> Do you really think that it was the last unsafe shell command in the Org 
> code?

No, but I prefer concrete examples. The CVE you linked to refers to
an already fixed bug.

> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/tree/lisp/ob-ditaa.el#n101
> and (shell-command pdf-cmd) below

The only unsafe part there is `java' that is taken from :java header
argument. I am unsure how to fix this issue without feature regression.

> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/tree/lisp/ob-lilypond.el#n194

May you provide a patch?

>>> I suppose, the issue has been received too much attention already:
>>>
>>> - https://security-tracker.debian.org/tracker/CVE-2023-28617
>>> - https://ubuntu.com/security/notices/USN-6003-1
>>> - https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2023-28617
>> 
>> These appear to be different issues.
>
> Linux distributions had to react to the CVE with formally high score and 
> updated Emacs packages applying 2 tiny patches from this thread.

Sure, but I have applied both the patches onto bugfix. That's why I
asked you what else we should do.

-- 
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>