Merge pull request #305 from yuchen0cc/main

untar: ignore '/' in tarball
containerd · Jan 9, 2024 · 4064710 · 4064710
2 parents ea97762 + 702b608
commit 4064710
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 13 deletions.
diff --git a/src/overlaybd/tar/libtar.cpp b/src/overlaybd/tar/libtar.cpp
@@ -125,11 +125,20 @@ int UnTar::extract_all() {
     dirs.clear();
 
     while ((i = read_header()) == 0) {
-        if (extract_file() != 0) {
-            LOG_ERRNO_RETURN(0, -1, "extract failed, filename `", get_pathname());
+        auto name = get_pathname(); // cleaned name
+        if (name == nullptr) {
+            LOG_ERRNO_RETURN(0, -1, "get filename failed");
+        }
+        if (strcmp(name, "/") == 0) {
+            LOG_WARN("file '/' ignored: resolved to root");
+            continue;
+        }
+        std::string filename(name);
+        if (extract_file(filename.c_str()) != 0) {
+            LOG_ERRNO_RETURN(0, -1, "extract failed, filename `", filename);
         }
         if (TH_ISDIR(header)) {
-            dirs.emplace_back(std::make_pair(std::string(get_pathname()), header.get_mtime()));
+            dirs.emplace_back(std::make_pair(filename, header.get_mtime()));
         }
         count++;
     }
@@ -150,11 +159,8 @@ int UnTar::extract_all() {
     return (i == 1 ? 0 : -1);
 }
 
-int UnTar::extract_file() {
+int UnTar::extract_file(const char *filename) {
     int i;
-    // normalize name
-    std::string npath = remove_last_slash(get_pathname());
-    const char *filename = npath.c_str();
 
     // ensure parent directory exists or is created.
     photon::fs::Path p(filename);
@@ -173,18 +179,18 @@ int UnTar::extract_file() {
 
     // check file exist
     struct stat s;
-    if (fs->lstat(npath.c_str(), &s) == 0 || errno != ENOENT) {
+    if (fs->lstat(filename, &s) == 0 || errno != ENOENT) {
         if (BIT_ISSET(options, TAR_NOOVERWRITE)) {
             errno = EEXIST;
             return -1;
         } else {
             if (!S_ISDIR(s.st_mode)) {
-                if (fs->unlink(npath.c_str()) == -1 && errno != ENOENT) {
-                    LOG_ERRNO_RETURN(EEXIST, -1, "remove exist file ` failed", npath.c_str());
+                if (fs->unlink(filename) == -1 && errno != ENOENT) {
+                    LOG_ERRNO_RETURN(EEXIST, -1, "remove exist file ` failed", filename);
                 }
             } else if (!TH_ISDIR(header)) {
-                if (remove_all(npath) == -1) {
-                    LOG_ERRNO_RETURN(EEXIST, -1, "remove exist dir ` failed", npath.c_str());
+                if (remove_all(filename) == -1) {
+                    LOG_ERRNO_RETURN(EEXIST, -1, "remove exist dir ` failed", filename);
                 }
             }
         }

diff --git a/src/overlaybd/tar/libtar.h b/src/overlaybd/tar/libtar.h
@@ -210,7 +210,7 @@ class UnTar : public TarCore {
     std::set<std::string> unpackedPaths;
     std::list<std::pair<std::string, int>> dirs; // <path, utime>
 
-    int extract_file();
+    int extract_file(const char *filename);
     int extract_regfile(const char *filename);
     int extract_regfile_meta_only(const char *filename);
     int extract_hardlink(const char *filename);

diff --git a/src/overlaybd/tar/test/test.cpp b/src/overlaybd/tar/test/test.cpp
@@ -372,6 +372,52 @@ TEST_F(TarTest, tar_header_check) {
     EXPECT_EQ(FILE_SIZE, ret);
 }
 
+TEST(CleanNameTest, clean_name) {
+    char name[256] = {0};
+    char *cname;
+    // 1. Reduce multiple slashes to a single slash.
+    strcpy(name, "/tar_test///busybox");
+    cname = clean_name(name);
+    EXPECT_EQ(0, strcmp(cname, "/tar_test/busybox"));
+    // 2. Eliminate . path name elements (the current directory).
+    strcpy(name, "/tar_test/./busybox");
+    cname = clean_name(name);
+    EXPECT_EQ(0, strcmp(cname, "/tar_test/busybox"));
+    // 3. Eliminate .. path name elements (the parent directory) and the non-. non-.., element that precedes them.
+    strcpy(name, "/tar_test/bin/../busybox");
+    cname = clean_name(name);
+    EXPECT_EQ(0, strcmp(cname, "/tar_test/busybox"));
+    strcpy(name, "/tar_test/bin/./../busybox");
+    cname = clean_name(name);
+    EXPECT_EQ(0, strcmp(cname, "/tar_test/busybox"));
+    strcpy(name, "/tar_test/test/bin/./../../busybox");
+    cname = clean_name(name);
+    EXPECT_EQ(0, strcmp(cname, "/tar_test/busybox"));
+    // 4. Eliminate .. elements that begin a rooted path, that is, replace /.. by / at the beginning of a path.
+    strcpy(name, "/.././tar_test/./test/bin/../busybox");
+    cname = clean_name(name);
+    EXPECT_EQ(0, strcmp(cname, "/tar_test/test/busybox"));
+    // 5. Leave intact .. elements that begin a non-rooted path.
+    strcpy(name, ".././tar_test/./test/bin/../busybox");
+    cname = clean_name(name);
+    EXPECT_EQ(0, strcmp(cname, "../tar_test/test/busybox"));
+    // If the result of this process is a null string, cleanname returns the string ".", representing the current directory.
+    strcpy(name, "");
+    cname = clean_name(name);
+    EXPECT_EQ(0, strcmp(cname, "."));
+    strcpy(name, "./");
+    cname = clean_name(name);
+    EXPECT_EQ(0, strcmp(cname, "."));
+    // root is remained
+    strcpy(name, "/");
+    cname = clean_name(name);
+    EXPECT_EQ(0, strcmp(cname, "/"));
+    // tailing '/' is removed
+    strcpy(name, "tar_test/");
+    cname = clean_name(name);
+    EXPECT_EQ(0, strcmp(cname, "tar_test"));
+}
+
 int main(int argc, char **argv) {
 
     ::testing::InitGoogleTest(&argc, argv);