unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: [Emacs-diffs] /srv/bzr/emacs/trunk r111747: * doc-view.el (doc-view-odf->pdf-converter-soffice): Use separate
       [not found] <E1U5DrQ-0008KT-9G@vcs.savannah.gnu.org>
@ 2013-02-12 14:21 ` Stefan Monnier
  2013-02-12 23:43   ` Andreas Schwab
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Monnier @ 2013-02-12 14:21 UTC (permalink / raw)
  To: Tassilo Horn; +Cc: emacs-devel

> +			   ;; A workaround is to start soffice with a
> +			   ;; separate UserInstallation directory.
> +			   (concat "-env:UserInstallation=file://"
> +				   (expand-file-name (format "libreoffice-docview%d" (user-uid))
> +						     temporary-file-directory))

I'm pretty sure there's some kind of race condition here that lets an
attacker on the same machine make you write files you didn't intend.

Also what if you have two doc-view buffers showing different ODF documents?
Will one soffice process cause the other to return immediately as well?

I think we should either use make-temp-file to create the
UserInstallation directory, or just reuse the directory in which we'll
place the png files.


        Stefan



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

* Re: [Emacs-diffs] /srv/bzr/emacs/trunk r111747: * doc-view.el (doc-view-odf->pdf-converter-soffice): Use separate
  2013-02-12 14:21 ` [Emacs-diffs] /srv/bzr/emacs/trunk r111747: * doc-view.el (doc-view-odf->pdf-converter-soffice): Use separate Stefan Monnier
@ 2013-02-12 23:43   ` Andreas Schwab
  2013-02-13  7:07     ` Tassilo Horn
  0 siblings, 1 reply; 6+ messages in thread
From: Andreas Schwab @ 2013-02-12 23:43 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel, Tassilo Horn

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> +			   ;; A workaround is to start soffice with a
>> +			   ;; separate UserInstallation directory.
>> +			   (concat "-env:UserInstallation=file://"
>> +				   (expand-file-name (format "libreoffice-docview%d" (user-uid))
>> +						     temporary-file-directory))
>
> I'm pretty sure there's some kind of race condition here that lets an
> attacker on the same machine make you write files you didn't intend.

It should be put in a directory like server.el and doc-view.el do.
server-ensure-safe-dir and doc-view-make-safe-dir should probably be
factored out into a generic function.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."



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

* Re: [Emacs-diffs] /srv/bzr/emacs/trunk r111747: * doc-view.el (doc-view-odf->pdf-converter-soffice): Use separate
  2013-02-12 23:43   ` Andreas Schwab
@ 2013-02-13  7:07     ` Tassilo Horn
  2013-02-13  8:31       ` Andreas Schwab
  0 siblings, 1 reply; 6+ messages in thread
From: Tassilo Horn @ 2013-02-13  7:07 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Stefan Monnier, emacs-devel

Andreas Schwab <schwab@linux-m68k.org> writes:

Hi!

>>> +			   ;; A workaround is to start soffice with a
>>> +			   ;; separate UserInstallation directory.
>>> +			   (concat "-env:UserInstallation=file://"
>>> + (expand-file-name (format "libreoffice-docview%d" (user-uid))
>>> +						     temporary-file-directory))
>>
>> I'm pretty sure there's some kind of race condition here that lets an
>> attacker on the same machine make you write files you didn't intend.
>> Also what if you have two doc-view buffers showing different ODF documents?
>> Will one soffice process cause the other to return immediately as well?

Yes, right.  Shortly after the change you cited, I've handled the latter
problem.  I tested that it works by marking several MS Office/ODF files
in dired and opening them all at once using F (from dired-x).  It works
flawlessly.

--8<---------------cut here---------------start------------->8---
(defun doc-view-odf->pdf-converter-soffice (odf callback)
  "Convert ODF to PDF asynchronously and call CALLBACK when finished.
The converted PDF is put into the current cache directory, and it
is named like ODF with the extension turned to pdf."
  ;; FIXME: soffice doesn't work when there's another running
  ;; LibreOffice instance, in which case it returns success without
  ;; actually doing anything.  See LibreOffice bug
  ;; https://bugs.freedesktop.org/show_bug.cgi?id=37531.  A workaround
  ;; is to start soffice with a separate UserInstallation directory.
  (let ((tmp-user-install-dir (make-temp-file "libreoffice-docview" t)))
    (doc-view-start-process "odf->pdf" doc-view-odf->pdf-converter-program
			    (list
			     (concat "-env:UserInstallation=file://"
				     tmp-user-install-dir)
			     "--headless" "--convert-to" "pdf"
			     "--outdir" (doc-view-current-cache-dir) odf)
			    (lambda ()
			      (delete-directory tmp-user-install-dir t)
			      (funcall callback)))))
--8<---------------cut here---------------end--------------->8---

> It should be put in a directory like server.el and doc-view.el do.
> server-ensure-safe-dir and doc-view-make-safe-dir should probably be
> factored out into a generic function.

Is there still a problem with this?  LibreOffice doesn't write any
sensitive data into that temporary directory, and as you can see, it's
immediately deleted after the ODF->PDF conversion.

Bye,
Tassilo



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

* Re: [Emacs-diffs] /srv/bzr/emacs/trunk r111747: * doc-view.el (doc-view-odf->pdf-converter-soffice): Use separate
  2013-02-13  7:07     ` Tassilo Horn
@ 2013-02-13  8:31       ` Andreas Schwab
  2013-02-13  9:14         ` Tassilo Horn
  0 siblings, 1 reply; 6+ messages in thread
From: Andreas Schwab @ 2013-02-13  8:31 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Tassilo Horn <tsdh@gnu.org> writes:

> Is there still a problem with this?  LibreOffice doesn't write any
> sensitive data into that temporary directory, and as you can see, it's
> immediately deleted after the ODF->PDF conversion.

It's not about sensive data, but about unsecure access to a
world-writable directory.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."



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

* Re: [Emacs-diffs] /srv/bzr/emacs/trunk r111747: * doc-view.el (doc-view-odf->pdf-converter-soffice): Use separate
  2013-02-13  8:31       ` Andreas Schwab
@ 2013-02-13  9:14         ` Tassilo Horn
  2013-02-13 15:04           ` Stefan Monnier
  0 siblings, 1 reply; 6+ messages in thread
From: Tassilo Horn @ 2013-02-13  9:14 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Stefan Monnier, emacs-devel

Andreas Schwab <schwab@linux-m68k.org> writes:

>> Is there still a problem with this?  LibreOffice doesn't write any
>> sensitive data into that temporary directory, and as you can see,
>> it's immediately deleted after the ODF->PDF conversion.
>
> It's not about sensive data, but about unsecure access to a
> world-writable directory.

Yes, I understand.  But the new version doesn't exhibit this problem
because (make-temp-file dir t) is guaranteed to create a new directory
(owned by the user with #o700 permissions), or do I miss something?

Bye,
Tassilo



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

* Re: [Emacs-diffs] /srv/bzr/emacs/trunk r111747: * doc-view.el (doc-view-odf->pdf-converter-soffice): Use separate
  2013-02-13  9:14         ` Tassilo Horn
@ 2013-02-13 15:04           ` Stefan Monnier
  0 siblings, 0 replies; 6+ messages in thread
From: Stefan Monnier @ 2013-02-13 15:04 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: emacs-devel

> Yes, I understand.  But the new version doesn't exhibit this problem
> because (make-temp-file dir t) is guaranteed to create a new directory
> (owned by the user with #o700 permissions), or do I miss something?

Yes, that's fine, thanks.


        Stefan



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

end of thread, other threads:[~2013-02-13 15:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <E1U5DrQ-0008KT-9G@vcs.savannah.gnu.org>
2013-02-12 14:21 ` [Emacs-diffs] /srv/bzr/emacs/trunk r111747: * doc-view.el (doc-view-odf->pdf-converter-soffice): Use separate Stefan Monnier
2013-02-12 23:43   ` Andreas Schwab
2013-02-13  7:07     ` Tassilo Horn
2013-02-13  8:31       ` Andreas Schwab
2013-02-13  9:14         ` Tassilo Horn
2013-02-13 15:04           ` Stefan Monnier

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

	https://git.savannah.gnu.org/cgit/emacs.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).