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

2023-05-12 Thread Ihor Radchenko
Max Nikulin  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 .
Support Org development at ,
or support my work at 



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

2023-05-11 Thread 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.)

2023-05-02 Thread Ihor Radchenko
Max Nikulin  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 .
Support Org development at ,
or support my work at 



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

2023-05-02 Thread 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: [PATCH] Fix ob-latex.el command injection vulnerability.

2023-05-01 Thread Ihor Radchenko
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?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2023-05-01 Thread 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.

2023-03-12 Thread Ihor Radchenko
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

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 .
Support Org development at ,
or support my work at 



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

2023-03-11 Thread lux
On Sat, 2023-03-11 at 10:47 +, 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.
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

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

2023-03-11 Thread Ihor Radchenko
lux  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 .
Support Org development at ,
or support my work at 



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

2023-03-10 Thread lux
On Thu, 2023-03-09 at 12:22 +, 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.

2023-03-09 Thread 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.

2023-03-09 Thread Ihor Radchenko
lux  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 .
Support Org development at ,
or support my work at 



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

2023-03-08 Thread lux
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 +, 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.
From 62f9d32decdd078633e51ea9fa30fdb000b6de51 Mon Sep 17 00:00:00 2001
From: Xi Lu 
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.

2023-03-07 Thread 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.

2023-03-07 Thread Max Nikulin

On 06/03/2023 10:17, lux wrote:

On Sat, 2023-02-18 at 11:43 +, 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.

2023-03-07 Thread Ihor Radchenko
Bastien Guerry  writes:

> Hi,
>
> Ihor Radchenko  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 .
Support Org development at ,
or support my work at 



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

2023-03-07 Thread Bastien Guerry
Hi,

Ihor Radchenko  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.

2023-03-07 Thread Ihor Radchenko
lux  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 .
Support Org development at ,
or support my work at 



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

2023-03-07 Thread lux
On Tue, 2023-03-07 at 12:35 +, Ihor Radchenko wrote:
> lux  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.

2023-03-07 Thread Ihor Radchenko
lux  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 .
Support Org development at ,
or support my work at 



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

2023-03-05 Thread lux
On Sat, 2023-02-18 at 11:43 +, Ihor Radchenko wrote:
> lux  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.
From adc0c558b1b091bb4bef77901633f31344b7391a Mon Sep 17 00:00:00 2001
From: Xi Lu 
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.

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

2023-02-18 Thread Ihor Radchenko
lux  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 .
Support Org development at ,
or support my work at 



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

2023-02-18 Thread lux
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.
From adc0c558b1b091bb4bef77901633f31344b7391a Mon Sep 17 00:00:00 2001
From: Xi Lu 
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.

2023-02-18 Thread 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