Re: [Emacs-diffs] /srv/bzr/emacs/trunk r111747: * doc-view.el (doc-view-odf->pdf-converter-soffice): Use separate

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r111747: * doc-view.el (doc-view-odf->pdf-converter-soffice): Use separate

[Stefan Monnier]

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

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r111747: * doc-view.el (doc-view-odf->pdf-converter-soffice): Use separate

[Andreas Schwab]

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

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r111747: * doc-view.el (doc-view-odf->pdf-converter-soffice): Use separate

[Tassilo Horn]

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

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r111747: * doc-view.el (doc-view-odf->pdf-converter-soffice): Use separate

[Andreas Schwab]

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

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r111747: * doc-view.el (doc-view-odf->pdf-converter-soffice): Use separate

[Tassilo Horn]

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

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r111747: * doc-view.el (doc-view-odf->pdf-converter-soffice): Use separate

[Stefan Monnier]

> 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