code.ossystems Code Review - openembedded-core.git/blob

   1 From 59357157706d47c365b2227739e17daba3607526 Mon Sep 17 00:00:00 2001
   2 From: Alessandro Ghedini <alessandro@ghedini.me>
   3 Date: Sun, 1 Mar 2015 12:07:45 +0100
   4 Subject: [PATCH] Add ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS option
   5
   6 This fixes a directory traversal in the cpio tool.
   7
   8
   9 Upstream-Status: backport
  10
  11 Signed-off-by: Li Zhou <li.zhou@windriver.com>
  12 ---
  13  cpio/bsdcpio.1                           |    3 ++-
  14  cpio/cpio.c                              |    2 ++
  15  libarchive/archive.h                     |    2 ++
  16  libarchive/archive_write_disk.3          |    3 +++
  17  libarchive/archive_write_disk_posix.c    |   14 +++++++++++---
  18  libarchive/test/test_write_disk_secure.c |   23 +++++++++++++++++++++++
  19  6 files changed, 43 insertions(+), 4 deletions(-)
  20
  21 diff --git a/cpio/bsdcpio.1 b/cpio/bsdcpio.1
  22 index f966aa0..e52546e 100644
  23 --- a/cpio/bsdcpio.1
  24 +++ b/cpio/bsdcpio.1
  25 @@ -156,7 +156,8 @@ See above for description.
  26  .It Fl Fl insecure
  27  (i and p mode only)
  28  Disable security checks during extraction or copying.
  29 -This allows extraction via symbolic links and path names containing
  30 +This allows extraction via symbolic links, absolute paths,
  31 +and path names containing
  32  .Sq ..
  33  in the name.
  34  .It Fl J , Fl Fl xz
  35 diff --git a/cpio/cpio.c b/cpio/cpio.c
  36 index 0acde11..b267e9b 100644
  37 --- a/cpio/cpio.c
  38 +++ b/cpio/cpio.c
  39 @@ -171,6 +171,7 @@ main(int argc, char *argv[])
  40         cpio->extract_flags |= ARCHIVE_EXTRACT_NO_OVERWRITE_NEWER;
  41         cpio->extract_flags |= ARCHIVE_EXTRACT_SECURE_SYMLINKS;
  42         cpio->extract_flags |= ARCHIVE_EXTRACT_SECURE_NODOTDOT;
  43 +       cpio->extract_flags |= ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS;
  44         cpio->extract_flags |= ARCHIVE_EXTRACT_PERM;
  45         cpio->extract_flags |= ARCHIVE_EXTRACT_FFLAGS;
  46         cpio->extract_flags |= ARCHIVE_EXTRACT_ACL;
  47 @@ -256,6 +257,7 @@ main(int argc, char *argv[])
  48                 case OPTION_INSECURE:
  49                         cpio->extract_flags &= ~ARCHIVE_EXTRACT_SECURE_SYMLINKS;
  50                         cpio->extract_flags &= ~ARCHIVE_EXTRACT_SECURE_NODOTDOT;
  51 +                       cpio->extract_flags &= ~ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS;
  52                         break;
  53                 case 'L': /* GNU cpio */
  54                         cpio->option_follow_links = 1;
  55 diff --git a/libarchive/archive.h b/libarchive/archive.h
  56 index 1f0fc38..ef635ac 100644
  57 --- a/libarchive/archive.h
  58 +++ b/libarchive/archive.h
  59 @@ -649,6 +649,8 @@ __LA_DECL int archive_read_set_passphrase_callback(struct archive *,
  60  /* Default: Do not use HFS+ compression if it was not compressed. */
  61  /* This has no effect except on Mac OS v10.6 or later. */
  62  #define        ARCHIVE_EXTRACT_HFS_COMPRESSION_FORCED  (0x8000)
  63 +/* Default: Do not reject entries with absolute paths */
  64 +#define ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS (0x10000)
  65
  66  __LA_DECL int archive_read_extract(struct archive *, struct archive_entry *,
  67                      int flags);
  68 diff --git a/libarchive/archive_write_disk.3 b/libarchive/archive_write_disk.3
  69 index fa925cc..a2e7afa 100644
  70 --- a/libarchive/archive_write_disk.3
  71 +++ b/libarchive/archive_write_disk.3
  72 @@ -177,6 +177,9 @@ The default is to not refuse such paths.
  73  Note that paths ending in
  74  .Pa ..
  75  always cause an error, regardless of this flag.
  76 +.It Cm ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS
  77 +Refuse to extract an absolute path.
  78 +The default is to not refuse such paths.
  79  .It Cm ARCHIVE_EXTRACT_SPARSE
  80  Scan data for blocks of NUL bytes and try to recreate them with holes.
  81  This results in sparse files, independent of whether the archive format
  82 diff --git a/libarchive/archive_write_disk_posix.c b/libarchive/archive_write_disk_posix.c
  83 index ab3bdac..c1290eb 100644
  84 --- a/libarchive/archive_write_disk_posix.c
  85 +++ b/libarchive/archive_write_disk_posix.c
  86 @@ -2509,8 +2509,9 @@ cleanup_pathname_win(struct archive_write_disk *a)
  87  /*
  88   * Canonicalize the pathname.  In particular, this strips duplicate
  89   * '/' characters, '.' elements, and trailing '/'.  It also raises an
  90 - * error for an empty path, a trailing '..' or (if _SECURE_NODOTDOT is
  91 - * set) any '..' in the path.
  92 + * error for an empty path, a trailing '..', (if _SECURE_NODOTDOT is
  93 + * set) any '..' in the path or (if ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS
  94 + * is set) if the path is absolute.
  95   */
  96  static int
  97  cleanup_pathname(struct archive_write_disk *a)
  98 @@ -2529,8 +2530,15 @@ cleanup_pathname(struct archive_write_disk *a)
  99         cleanup_pathname_win(a);
 100  #endif
 101         /* Skip leading '/'. */
 102 -       if (*src == '/')
 103 +       if (*src == '/') {
 104 +               if (a->flags & ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS) {
 105 +                       archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
 106 +                                         "Path is absolute");
 107 +                       return (ARCHIVE_FAILED);
 108 +               }
 109 +
 110                 separator = *src++;
 111 +       }
 112
 113         /* Scan the pathname one element at a time. */
 114         for (;;) {
 115 diff --git a/libarchive/test/test_write_disk_secure.c b/libarchive/test/test_write_disk_secure.c
 116 index 31c5bfd..2c94206 100644
 117 --- a/libarchive/test/test_write_disk_secure.c
 118 +++ b/libarchive/test/test_write_disk_secure.c
 119 @@ -178,6 +178,29 @@ DEFINE_TEST(test_write_disk_secure)
 120         assert(S_ISDIR(st.st_mode));
 121         archive_entry_free(ae);
 122
 123 +       /*
 124 +        * Without security checks, we should be able to
 125 +        * extract an absolute path.
 126 +        */
 127 +       assert((ae = archive_entry_new()) != NULL);
 128 +       archive_entry_copy_pathname(ae, "/tmp/libarchive_test-test_write_disk_secure-absolute_path.tmp");
 129 +       archive_entry_set_mode(ae, S_IFREG | 0777);
 130 +       assert(0 == archive_write_header(a, ae));
 131 +       assert(0 == archive_write_finish_entry(a));
 132 +       assertFileExists("/tmp/libarchive_test-test_write_disk_secure-absolute_path.tmp");
 133 +       assert(0 == unlink("/tmp/libarchive_test-test_write_disk_secure-absolute_path.tmp"));
 134 +
 135 +       /* But with security checks enabled, this should fail. */
 136 +       assert(archive_entry_clear(ae) != NULL);
 137 +       archive_entry_copy_pathname(ae, "/tmp/libarchive_test-test_write_disk_secure-absolute_path.tmp");
 138 +       archive_entry_set_mode(ae, S_IFREG | 0777);
 139 +       archive_write_disk_set_options(a, ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS);
 140 +       failure("Extracting an absolute path should fail here.");
 141 +       assertEqualInt(ARCHIVE_FAILED, archive_write_header(a, ae));
 142 +       archive_entry_free(ae);
 143 +       assert(0 == archive_write_finish_entry(a));
 144 +       assertFileNotExists("/tmp/libarchive_test-test_write_disk_secure-absolute_path.tmp");
 145 +
 146         assertEqualInt(ARCHIVE_OK, archive_write_free(a));
 147
 148         /* Test the entries on disk. */
 149 --
 150 1.7.9.5
 151