Browse Source

Fix insecure file permissions when using FileFd with OpenMode::Atomic

Commit 7335eebea6 introduced a bug
that caused FileFd to create insecure permissions when FileFd::Atomic
is used. This commit fixes the permissions and adds a test.

The bug is most likely caused by the confusing "Perm" parameter
that is passed to Open() - its not the file permissions but intead
the "mode" part of open/creat.
debian/1.8.y
Michael Vogt 8 years ago
parent
commit
f22b65b479
  1. 8
      apt-pkg/contrib/fileutl.cc
  2. 31
      test/libapt/fileutl_test.cc

8
apt-pkg/contrib/fileutl.cc

@ -1067,6 +1067,10 @@ bool FileFd::Open(string FileName,unsigned int const Mode,APT::Configuration::Co
if_FLAGGED_SET(Exclusive, O_EXCL);
#undef if_FLAGGED_SET
// there is no getumask() so we read it by setting it and reset
mode_t current_umask = umask(0);
umask(current_umask);
if ((Mode & Atomic) == Atomic)
{
char *name = strdup((FileName + ".XXXXXX").c_str());
@ -1080,11 +1084,11 @@ bool FileFd::Open(string FileName,unsigned int const Mode,APT::Configuration::Co
TemporaryFileName = string(name);
free(name);
if(Perms != 600 && fchmod(iFd, Perms) == -1)
if(Perms != 600 && fchmod(iFd, Perms & ~current_umask) == -1)
return FileFdErrno("fchmod", "Could not change permissions for temporary file %s", TemporaryFileName.c_str());
}
else
iFd = open(FileName.c_str(), fileflags, Perms);
iFd = open(FileName.c_str(), fileflags, Perms & ~current_umask);
this->FileName = FileName;
if (iFd == -1 || OpenInternDescriptor(Mode, compressor) == false)

31
test/libapt/fileutl_test.cc

@ -6,13 +6,44 @@
#include <string>
#include <vector>
#include <stdlib.h>
#include <sys/stat.h>
#include "assert.h"
// regression test for permission bug LP: #1304657
static bool
TestFileFdOpenPermissions(mode_t a_umask, mode_t ExpectedFilePermission)
{
FileFd f;
struct stat buf;
static const char* fname = "test.txt";
umask(a_umask);
f.Open(fname, FileFd::ReadWrite|FileFd::Atomic);
f.Close();
if (stat(fname, &buf) < 0)
{
_error->Errno("stat", "failed to stat");
_error->DumpErrors();
return false;
}
unlink(fname);
equals(buf.st_mode & 0777, ExpectedFilePermission);
return true;
}
int main()
{
std::vector<std::string> files;
if (TestFileFdOpenPermissions(0002, 0664) == false ||
TestFileFdOpenPermissions(0022, 0644) == false ||
TestFileFdOpenPermissions(0077, 0600) == false ||
TestFileFdOpenPermissions(0026, 0640) == false)
{
return 1;
}
// normal match
files = Glob("*.lst");
if (files.size() != 1)

Loading…
Cancel
Save