don't change owner/perms/times through file:// symlinks

If we have files in partial/ from a previous invocation or similar such
those could be symlinks created by file:// sources. The code is
expecting only real files through and happily changes owner,
modification times and permission on the file the symlink points to
which tend to be files we have no business in touching in this way.
Permissions of symlinks shouldn't be changed, changing owner is usually
pointless to, but just to be sure we pick the easy way out and use
lchown, check for symlinks before chmod/utimes.

Reported-By: Mattia Rizzolo on IRC
(cherry picked from commit 3465138575e1fd0d5892d9b6be1ae232eb873460)

diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc
index 09dce41239d301916d6125c69888526e82baa8ca..ef88f6fddd73de89526d7643a52aa9d99a266785 100644 (file)
--- a/apt-pkg/contrib/fileutl.cc
+++ b/apt-pkg/contrib/fileutl.cc
@@ -916,9 +916,12 @@ bool ChangeOwnerAndPermissionOfFile(char const * const requester, char const * c
       // ensure the file is owned by root and has good permissions
       struct passwd const * const pw = getpwnam(user);
       struct group const * const gr = getgrnam(group);
-      if (pw != NULL && gr != NULL && chown(file, pw->pw_uid, gr->gr_gid) != 0)
+      if (pw != NULL && gr != NULL && lchown(file, pw->pw_uid, gr->gr_gid) != 0)
         Res &= _error->WarningE(requester, "chown to %s:%s of file %s failed", user, group, file);
    }
+   struct stat Buf;
+   if (lstat(file, &Buf) != 0 || S_ISLNK(Buf.st_mode))
+      return Res;
    if (chmod(file, mode) != 0)
       Res &= _error->WarningE(requester, "chmod 0%o of file %s failed", mode, file);
    return Res;

diff --git a/methods/aptmethod.h b/methods/aptmethod.h
index f8a68c92bae34218b651ef7b49c7ee55e4a3a789..dd488fe6f5bd62eda9710d92b929ea830eb1741b 100644 (file)
--- a/methods/aptmethod.h
+++ b/methods/aptmethod.h
@@ -3,9 +3,17 @@
 
 #include <apt-pkg/acquire-method.h>
 #include <apt-pkg/configuration.h>
+#include <apt-pkg/error.h>
 
 #include <string>
 
+#include <sys/time.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#include <apti18n.h>
+
 class aptMethod : public pkgAcqMethod
 {
    char const * const Binary;
@@ -42,6 +50,29 @@ public:
       va_end(args);
    }
 
+   bool TransferModificationTimes(char const * const From, char const * const To, time_t &LastModified)
+   {
+      if (strcmp(To, "/dev/null") == 0)
+        return true;
+
+      struct stat Buf2;
+      if (lstat(To, &Buf2) != 0 || S_ISLNK(Buf2.st_mode))
+        return true;
+
+      struct stat Buf;
+      if (stat(From, &Buf) != 0)
+        return _error->Errno("stat",_("Failed to stat"));
+
+      // we don't use utimensat here for compatibility reasons: #738567
+      struct timeval times[2];
+      times[0].tv_sec = Buf.st_atime;
+      LastModified = times[1].tv_sec = Buf.st_mtime;
+      times[0].tv_usec = times[1].tv_usec = 0;
+      if (utimes(To, times) != 0)
+        return _error->Errno("utimes",_("Failed to set modification time"));
+      return true;
+   }
+
    aptMethod(char const * const Binary, char const * const Ver, unsigned long const Flags) :
       pkgAcqMethod(Ver, Flags), Binary(Binary)
    {}

diff --git a/methods/copy.cc b/methods/copy.cc
index e515b2def10e1eed04abd89709915703470ec684..49b1bae5e39ace24010d2f060e2f275998c28a12 100644 (file)
--- a/methods/copy.cc
+++ b/methods/copy.cc
@@ -79,13 +79,8 @@ bool CopyMethod::Fetch(FetchItem *Itm)
    From.Close();
    To.Close();
 
-   // Transfer the modification times
-   struct timeval times[2];
-   times[0].tv_sec = Buf.st_atime;
-   times[1].tv_sec = Buf.st_mtime;
-   times[0].tv_usec = times[1].tv_usec = 0;
-   if (utimes(Res.Filename.c_str(), times) != 0)
-      return _error->Errno("utimes",_("Failed to set modification time"));
+   if (TransferModificationTimes(File.c_str(), Res.Filename.c_str(), Res.LastModified) == false)
+      return false;
 
    CalculateHashes(Itm, Res);
    URIDone(Res);

diff --git a/methods/store.cc b/methods/store.cc
index 2ad0f0177675a6ac50e4ee8a7d2dbecdbfac3a86..515c633877f83c5779f25ea73cc05645ae692f08 100644 (file)
--- a/methods/store.cc
+++ b/methods/store.cc
@@ -127,20 +127,8 @@ bool StoreMethod::Fetch(FetchItem *Itm)                                    /*{{{*/
    if (Failed == true)
       return false;
 
-   // Transfer the modification times
-   if (Itm->DestFile != "/dev/null")
-   {
-      struct stat Buf;
-      if (stat(Path.c_str(),&Buf) != 0)
-        return _error->Errno("stat",_("Failed to stat"));
-
-      struct timeval times[2];
-      times[0].tv_sec = Buf.st_atime;
-      Res.LastModified = times[1].tv_sec = Buf.st_mtime;
-      times[0].tv_usec = times[1].tv_usec = 0;
-      if (utimes(Itm->DestFile.c_str(), times) != 0)
-        return _error->Errno("utimes",_("Failed to set modification time"));
-   }
+   if (TransferModificationTimes(Path.c_str(), Itm->DestFile.c_str(), Res.LastModified) == false)
+      return false;
 
    // Return a Done response
    Res.TakeHashes(Hash);

diff --git a/test/integration/framework b/test/integration/framework
index ca0a3b5deaddf651f3fcdb04b8d044663e3d2b19..7fdf21bed3455c39f539be05dec9129a14a3d245 100644 (file)
--- a/test/integration/framework
+++ b/test/integration/framework
@@ -1862,6 +1862,11 @@ pause() {
        read IGNORE
 }
 
+logcurrentarchivedirectory() {
+       find "${TMPWORKINGDIRECTORY}/aptarchive/dists" -type f | while read line; do
+               stat --format '%U:%G:%a:%n' "$line"
+       done | sort > "${TMPWORKINGDIRECTORY}/rootdir/var/log/aptgetupdate.before.lst"
+}
 listcurrentlistsdirectory() {
        {
                find rootdir/var/lib/apt/lists -maxdepth 1 -type d | while read line; do
@@ -1940,6 +1945,10 @@ aptautotest_aptget_update() {
                # failure cases can retain partial files and such
                testempty find "${TMPWORKINGDIRECTORY}/rootdir/var/lib/apt/lists/partial" -mindepth 1 ! \( -name 'lock' -o -name '*.FAILED' \)
        fi
+       if [ -s "${TMPWORKINGDIRECTORY}/rootdir/var/log/aptgetupdate.before.lst" ]; then
+               testfileequal "${TMPWORKINGDIRECTORY}/rootdir/var/log/aptgetupdate.before.lst" \
+                       "$(find "${TMPWORKINGDIRECTORY}/aptarchive/dists" -type f | while read line; do stat --format '%U:%G:%a:%n' "$line"; done | sort)"
+       fi
 }
 aptautotest_apt_update() { aptautotest_aptget_update "$@"; }
 aptautotest_aptcdrom_add() { aptautotest_aptget_update "$@"; }

diff --git a/test/integration/test-apt-update-file b/test/integration/test-apt-update-file
index 20f6046957fd95b36c2da7c6349b673c52ba1964..8da4ec35b352f3c883c38c92b75bf8a17db0f864 100755 (executable)
--- a/test/integration/test-apt-update-file
+++ b/test/integration/test-apt-update-file
@@ -18,6 +18,7 @@ insertpackage 'unstable' 'bar' 'amd64' '1'
 insertsource 'unstable' 'foo' 'all' '1'
 
 setupaptarchive --no-update
+logcurrentarchivedirectory
 
 # ensure the archive is not writable
 addtrap 'prefix' 'chmod 755 aptarchive/dists/unstable/main/binary-all;'
@@ -37,8 +38,11 @@ if [ "$(id -u)" = '0' ]; then
        rm -rf rootdir/var/lib/apt/lists
        chmod 500 aptarchive/dists/
        testsuccesswithnotice aptget update
-       exit
+       chmod 755 aptarchive/dists/
+else
+       testsuccess aptget update
 fi
+mv rootdir/var/lib/apt/lists/_* rootdir/var/lib/apt/lists/partial
 chmod 555 aptarchive/dists/unstable/main/binary-all
 testsuccess aptget update -o Debug::pkgAcquire::Worker=1
 cp -a rootdir/tmp/testsuccess.output rootdir/tmp/update.output

diff --git a/test/integration/test-apt-update-ims b/test/integration/test-apt-update-ims
index 241bf383bdc3b53108ab30c897a4c4021c430975..5a44911a6df8e8c73df660ce012a978e95c75ac1 100755 (executable)
--- a/test/integration/test-apt-update-ims
+++ b/test/integration/test-apt-update-ims
@@ -11,6 +11,7 @@ insertpackage 'unstable' 'unrelated2' 'amd64' '0.5~squeeze1'
 insertsource 'unstable' 'unrelated' 'all' '0.5~squeeze1'
 
 setupaptarchive --no-update
+logcurrentarchivedirectory
 changetowebserver
 
 runtest() {
@@ -70,6 +71,7 @@ EXPECT="Ign:1 http://localhost:${APTHTTPPORT} unstable InRelease
 Hit:2 http://localhost:${APTHTTPPORT} unstable Release
 Reading package lists..."
 find aptarchive -name 'InRelease' -delete
+logcurrentarchivedirectory
 echo 'Acquire::GzipIndexes "0";' > rootdir/etc/apt/apt.conf.d/02compressindex
 runtest
 echo 'Acquire::GzipIndexes "1";' > rootdir/etc/apt/apt.conf.d/02compressindex
@@ -86,6 +88,7 @@ W: The repository 'http://localhost:${APTHTTPPORT} unstable Release' is not sign
 N: Data from such a repository can't be authenticated and is therefore potentially dangerous to use.
 N: See apt-secure(8) manpage for repository creation and user configuration details."
 find aptarchive -name 'Release.gpg' -delete
+logcurrentarchivedirectory
 echo 'Acquire::GzipIndexes "0";' > rootdir/etc/apt/apt.conf.d/02compressindex
 runtest 'warning'
 echo 'Acquire::GzipIndexes "1";' > rootdir/etc/apt/apt.conf.d/02compressindex
@@ -98,6 +101,7 @@ find aptarchive -name '*Release' -exec sed -i \
        -e '/^Valid-Until: / d' -e "/^Date: / a\
 Valid-Until: $(date -d '-1 weeks' '+%a, %d %b %Y %H:%M:%S %Z')" '{}' \;
 signreleasefiles
+logcurrentarchivedirectory
 
 msgmsg 'expired InRelease'
 EXPECT="Hit:1 http://localhost:${APTHTTPPORT} unstable InRelease
@@ -115,6 +119,7 @@ Hit:2 http://localhost:${APTHTTPPORT} unstable Release
 Reading package lists...
 E: Release file for http://localhost:${APTHTTPPORT}/dists/unstable/Release is expired (invalid since). Updates for this repository will not be applied."
 find aptarchive -name 'InRelease' -delete
+logcurrentarchivedirectory
 echo 'Acquire::GzipIndexes "0";' > rootdir/etc/apt/apt.conf.d/02compressindex
 runtest 'failure'
 echo 'Acquire::GzipIndexes "1";' > rootdir/etc/apt/apt.conf.d/02compressindex
@@ -132,6 +137,7 @@ N: Data from such a repository can't be authenticated and is therefore potential
 N: See apt-secure(8) manpage for repository creation and user configuration details.
 E: Release file for http://localhost:${APTHTTPPORT}/dists/unstable/Release is expired (invalid since). Updates for this repository will not be applied."
 find aptarchive -name 'Release.gpg' -delete
+logcurrentarchivedirectory
 echo 'Acquire::GzipIndexes "0";' > rootdir/etc/apt/apt.conf.d/02compressindex
 runtest 'failure' 'warning'
 echo 'Acquire::GzipIndexes "1";' > rootdir/etc/apt/apt.conf.d/02compressindex
@@ -176,6 +182,7 @@ W: The repository 'http://localhost:${APTHTTPPORT} unstable Release' does not ha
 N: Data from such a repository can't be authenticated and is therefore potentially dangerous to use.
 N: See apt-secure(8) manpage for repository creation and user configuration details."
 find aptarchive -name '*Release*' -delete
+logcurrentarchivedirectory
 echo 'Acquire::GzipIndexes "0";
 Acquire::PDiffs "0";' > rootdir/etc/apt/apt.conf.d/02compressindex
 runtest 'warning'