bug#4291: 23.1; doc-view-mode temporary directory vulnerable to denial of service

bug#4291: 23.1; doc-view-mode temporary directory vulnerable to denial of service

[David Bremner]

By default doc-view-mode makes a directory /tmp/docview$uid .  Since
this is easily predictable, a malicious person could cause docview to
fail simply by creating a directory with the same name. 

In GNU Emacs 23.1.1 (i486-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2009-08-03 on raven, modified by Debian
Windowing system distributor `The X.Org Foundation', version 11.0.10603000
configured using `configure  '--build=i486-linux-gnu' '--host=i486-linux-gnu' '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib' '--localstatedir=/var/lib' '--infodir=/usr/share/info' '--mandir=/usr/share/man' '--with-pop=yes' '--enable-locallisppath=/etc/emacs23:/etc/emacs:/usr/local/share/emacs/23.1/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/23.1/site-lisp:/usr/share/emacs/site-lisp:/usr/share/emacs/23.1/leim' '--with-x=yes' '--with-x-toolkit=athena' '--with-toolkit-scroll-bars' 'build_alias=i486-linux-gnu' 'host_alias=i486-linux-gnu' 'CFLAGS=-DDEBIAN -g -O2' 'LDFLAGS=-g' 'CPPFLAGS=''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_CA.UTF-8
  value of $XMODIFIERS: nil
  locale-coding-system: utf-8-unix
  default-enable-multibyte-characters: t

Major mode: Fundamental

Minor modes in effect:
  diff-auto-refine-mode: t
  tool-bar-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  global-auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
M-x C-g C-x C-f t e a SPC c s SPC 2 SPC SPC w i SPC 
c SPC SPC SPC SPC <return> M-x r e p SPC o SPC SPC 
r SPC SPC SPC <return>

Recent messages:
Loading /home/bremner/.emacs-custom.el (source)...
Loading epa-mail...done
Loading /home/bremner/.emacs-custom.el (source)...done
Loading /usr/share/emacs/site-lisp/haskell-mode/haskell-site-file.el (source)...done
For information about GNU Emacs and the GNU system, type C-h C-a.
Quit
Making completion list... [2 times]
File mode specification error: (file-error "Doing chmod" "operation not permitted" "/tmp/docview1000")
Loading vc-git...done
Making completion list... [3 times]

bug#4291: 23.1; doc-view-mode temporary directory vulnerable to denial of service

[Glenn Morris]

David Bremner wrote:

> By default doc-view-mode makes a directory /tmp/docview$uid .  Since
> this is easily predictable, a malicious person could cause docview to
> fail simply by creating a directory with the same name. 

Couldn't they do the same thing by simply filling /tmp with junk, no
matter what filename is used?

(Emacs server also uses the same name every time AFAIK.)

bug#4291: 23.1; doc-view-mode temporary directory vulnerable to denial of service

[Stefan Monnier]

>> By default doc-view-mode makes a directory /tmp/docview$uid .  Since
>> this is easily predictable, a malicious person could cause docview to
>> fail simply by creating a directory with the same name.
> Couldn't they do the same thing by simply filling /tmp with junk, no
> matter what filename is used?

Yes, tho it's a bit different: your case can be avoided by appropriate
use of quotas on /tmp (yes, I realize this is highly unlikely), and your
case cannot be obtained without impacting the system as a whole
(i.e. it's less discrete).

> (Emacs server also uses the same name every time AFAIK.)

Yes, and Emacs server needs this name to be predictable (an "ls /tmp"
shows that other services, such as `orbit', are similarly vulnerable).

IIRC /tmp/docview$uid is predictable because doc-view tries to reuse
previouly-rendered pages.  I'm not convinced this is really a good
feature, but obviously the author thought it was important, so I'd
rather not drop it without a discussion.


        Stefan

bug#4291: 23.1; doc-view-mode temporary directory vulnerable to denial of service

[Glenn Morris]

Stefan Monnier wrote:

> Yes, tho it's a bit different: your case can be avoided by appropriate
> use of quotas on /tmp (yes, I realize this is highly unlikely), and your
> case cannot be obtained without impacting the system as a whole
> (i.e. it's less discrete).

The original scenario doesn't seem likely (or discreet). I suggest
just making docview give an explicit error if its cache dir: a) cannot
be created; or b) exists but cannot be read or written to.

bug#4291: 23.1; doc-view-mode temporary directory vulnerable to denial of service

[Lars Magne Ingebrigtsen]

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

> IIRC /tmp/docview$uid is predictable because doc-view tries to reuse
> previouly-rendered pages.  I'm not convinced this is really a good
> feature, but obviously the author thought it was important, so I'd
> rather not drop it without a discussion.

It could just stash the directory name in a variable, and use the normal
`make-temp-file' to create the directory, couldn't it?

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

bug#4291: 23.1; doc-view-mode temporary directory vulnerable to denial of service

[Glenn Morris]

Lars Magne Ingebrigtsen wrote:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> IIRC /tmp/docview$uid is predictable because doc-view tries to reuse
>> previouly-rendered pages.  I'm not convinced this is really a good
>> feature, but obviously the author thought it was important, so I'd
>> rather not drop it without a discussion.
>
> It could just stash the directory name in a variable, and use the normal
> `make-temp-file' to create the directory, couldn't it?

I think the idea referred to above is to potentially re-use pages
converted by a previous Emacs instance (which seems like a bad feature
to me too).

bug#4291: 23.1; doc-view-mode temporary directory vulnerable to denial of service

[Lars Magne Ingebrigtsen]

Glenn Morris <rgm@gnu.org> writes:

>>> IIRC /tmp/docview$uid is predictable because doc-view tries to reuse
>>> previouly-rendered pages.  I'm not convinced this is really a good
>>> feature, but obviously the author thought it was important, so I'd
>>> rather not drop it without a discussion.
>>
>> It could just stash the directory name in a variable, and use the normal
>> `make-temp-file' to create the directory, couldn't it?
>
> I think the idea referred to above is to potentially re-use pages
> converted by a previous Emacs instance (which seems like a bad feature
> to me too).

Oh, I see.  Hm.  Sounds like a bad idea to me, too.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

bug#4291: 23.1; doc-view-mode temporary directory vulnerable to denial of service

[Chong Yidong]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

>>>> IIRC /tmp/docview$uid is predictable because doc-view tries to reuse
>>>> previouly-rendered pages.  I'm not convinced this is really a good
>>>> feature, but obviously the author thought it was important, so I'd
>>>> rather not drop it without a discussion.
>>>
>>> It could just stash the directory name in a variable, and use the normal
>>> `make-temp-file' to create the directory, couldn't it?
>>
>> I think the idea referred to above is to potentially re-use pages
>> converted by a previous Emacs instance (which seems like a bad feature
>> to me too).
>
> Oh, I see.  Hm.  Sounds like a bad idea to me, too.  :-)

I think the idea is that if you view the same pdf N times in Emacs, the
disk space won't increase monotonically (Emacs doesn't delete any of
those files AFAIK).

bug#4291: 23.1; doc-view-mode temporary directory vulnerable to denial of service

[Lars Magne Ingebrigtsen]

Chong Yidong <cyd@stupidchicken.com> writes:

> I think the idea is that if you view the same pdf N times in Emacs, the
> disk space won't increase monotonically (Emacs doesn't delete any of
> those files AFAIK).

If they aren't deleted, then using a single directory per UID seems like
a good choice.  Perhaps the right fix here is to make the error message
better? 

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

bug#4291: 23.1; doc-view-mode temporary directory vulnerable to denial of service

[Lars Magne Ingebrigtsen]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

>> I think the idea is that if you view the same pdf N times in Emacs, the
>> disk space won't increase monotonically (Emacs doesn't delete any of
>> those files AFAIK).
>
> If they aren't deleted, then using a single directory per UID seems like
> a good choice.  Perhaps the right fix here is to make the error message
> better? 

I've now done this.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/