[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#741627: insecure temporary file usage in apt-extracttemplates



Package: apt
Version: 0.9.7.9+deb7u1
Severity: important
Tags: security


When installing/upgrading packages via `apt-get` a child process
is invoked against the downloaded .deb-file to extract any templates
which might be contained in that package.

For example I was recently upgrading my lighttpd package and I see
this is logged (thanks to `snoopy`):

  apt-extracttemplates /var/cache/apt/archives/lighttpd_1.4.31-4+deb7u2_amd64.deb 

As that package contains no actual templates all is well.

However consider a case where templates/config files are present,
again upon my system I can see that some recent downloads do indeed
include such things, and they are output as expected via the
apt-extracttemplates invokation:

  shelob ~ $ apt-extracttemplates /var/cache/apt/archives/gdm3_3.4.1-8_amd64.deb 
  gdm3 3.4.1-8 /tmp/gdm3.template.136800 /tmp/gdm3.config.136801

What `apt-extracttemplates` has done is twofold:

  * Extracted the template & config files.
  * Reported their location.

However what it has also done is create files with predictable
filenames, overwriting the carefully constructed memoirs I kept
in the file /tmp/gdm3.template.136800 ... ;)

Mitigating factors?  The code correctly removes files first, so
symlinks and hardlinks are not followed.  I suppose that makes this
"trivial" rather than "serious", (Yes a "standard" temporary race
could allow symlink/link following, but it looks like the file
is opened via O_CREAT + O_EXCL so that's not a concern - right?)

Anyway given that the generated file names are output to the console
it feels like we should use mkstemp and do it properly, right?


The code in question is in cmdline/apt-extracttemplates.cc,
in the function "string WriteFile(const char *package, const char *prefix, const char *data)
":

	char fn[512];
...

	snprintf(fn, sizeof(fn), "%s/%s.%s.%u%d",
                 _config->Find("APT::ExtractTemplates::TempDir", tempdir).c_str(),
                 package, prefix, getpid(), i++);
....
	if (!f.Open(fn, FileFd::WriteTemp, 0600))


The opening, which mitigates this, is carried out using 'WriteTemp',
which is implemented in apt-pkg/contrib/fileutl.cc, and maps to:

  	WriteTemp = ReadWrite | Create | Exclusive,

(from fileutl.h.)


Steve
-- 
http://tweaked.io/

-- Package-specific info:

-- (no /etc/apt/preferences present) --


-- (/etc/apt/sources.list present, but not submitted) --


-- System Information:
Debian Release: 7.4
  APT prefers stable-updates
  APT policy: (500, 'stable-updates'), (500, 'stable')
Architecture: amd64 (x86_64)

Kernel: Linux 3.12-0.bpo.1-amd64 (SMP w/8 CPU cores)
Locale: LANG=en_US.UTF8, LC_CTYPE=en_US.UTF8 (charmap=UTF-8) (ignored: LC_ALL set to en_US.UTF8)
Shell: /bin/sh linked to /bin/dash

Versions of packages apt depends on:
ii  debian-archive-keyring  2012.4
ii  gnupg                   1.4.12-7+deb7u3
ii  libapt-pkg4.12          0.9.7.9+deb7u1
ii  libc6                   2.13-38+deb7u1
ii  libgcc1                 1:4.7.2-5
ii  libstdc++6              4.7.2-5

apt recommends no packages.

Versions of packages apt suggests:
pn  apt-doc     <none>
ii  aptitude    0.6.8.2-1
ii  dpkg-dev    1.16.12
ii  python-apt  0.8.8.2
ii  synaptic    0.75.13
ii  xz-utils    5.1.1alpha+20120614-2


Reply to: