Implemented requested changes

oss-internship-2020/libarchive/ld_preload_example/CMakeLists.txt

@@ -29,9 +29,6 @@ target_link_libraries(minitar_original PRIVATE
     minitar_lib
 )
 
-
-#===========
-
 add_library(sapi_minitar_lib_shared SHARED
   ../examples/sapi_minitar.cc
   ../examples/sapi_minitar.h
@@ -53,8 +50,6 @@ target_include_directories(sapi_minitar_lib_shared INTERFACE
   "${PROJECT_SOURCE_DIR}/examples"
 )
 
-#===========
-
 add_library(minitar_preload SHARED
   sapi_minitar.cc
 )

oss-internship-2020/libarchive/ld_preload_example/README.md

@@ -7,6 +7,7 @@ This example only implements the extract part of the original tool.
 ## Structure
 - **minitar_main.cc** - the original main function.
 - **minitar.cc** and **minitar.h** - original functions called from main, built into a shared library.
+- The three files mentioned above are taken from the original minitar [example](https://github.com/libarchive/libarchive/tree/master/examples/minitar). The only difference to the code is that the main function was separated from the rest in order to build a shared library so that when `LD_PRELOAD` is used we can call our custom functions.
 - **sapi_minitar.cc** - libarchive functions with our own implementation, built into a shared library which will be used with `LD_PRELOAD`.
 
 Most of the functions simply convert the arguments to sapi::v objects and calls the sandboxed version of the function. Also, there is a custom *extract* implementation that also created the sandbox object. If the sandbox could be created without special arguments then this would not be needed and could be done inside of the first function called. However, in our case, the sandbox requires the file path and so we do this in the *create* function where we have access to that path. Aftert this, we call the original function (the "next" function symbol).
@@ -21,5 +22,4 @@ The only changes are in the root CMakeLists.txt:
 ## Usage
 `LD_PRELOAD= build/ld_preload_example/libminitar_preload.so build/ld_preload_example/minitar_original -xvf archive_file`
 
-
 Instead of the *x* option, *t* can be used as well only to print the archive entries.

oss-internship-2020/libarchive/ld_preload_example/minitar.cc

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// clang-format off
 #include "minitar.h"  // NOLINT(build/include)
 
 /*
@@ -250,7 +251,6 @@ void extract(const char* filename, int do_extract, int flags, int verbose) {
 
   archive_write_close(ext);
   archive_write_free(ext);
-  exit(0);
 }
 
 int copy_data(struct archive* ar, struct archive* aw) {
@@ -308,3 +308,5 @@ void usage(void) {
   errmsg(m);
   exit(1);
 }
+
+// clang-format on

oss-internship-2020/libarchive/ld_preload_example/minitar.h

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// clang-format off
 #include <sys/types.h>
 #include <sys/stat.h>
 
@@ -98,3 +99,5 @@ void extract(const char *filename, int do_extract, int flags, int verbose = 1);
 int	copy_data(struct archive *, struct archive *);
 void msg(const char *);
 void usage(void);
+
+// clang-format on

oss-internship-2020/libarchive/ld_preload_example/minitar_main.cc

@@ -1,8 +1,18 @@
-/*-
- * This file is in the public domain.
- * Do with it as you will.
- */
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
 
+// clang-format off
 /*-
  * This is a compact "tar" program whose primary goal is small size.
  * Statically linked, it can be very small indeed.  This serves a number
@@ -193,3 +203,5 @@ int main(int unused_argc, const char** argv) {
 
   return (0);
 }
+
+// clang-format on

oss-internship-2020/libarchive/ld_preload_example/sapi_minitar.cc

@@ -24,20 +24,9 @@
 #include "sandboxed_api/var_array.h"
 #include "sandboxed_api/var_ptr.h"
 
-std::unique_ptr<SapiLibarchiveSandboxExtract> sandbox_extract;
-std::unique_ptr<LibarchiveApi> api;
-
-struct ExtractTempDirectoryCleanup {
-  ExtractTempDirectoryCleanup(const std::string& dir) : dir_(dir) {}
-  ~ExtractTempDirectoryCleanup() {
-    sandbox2::file_util::fileops::DeleteRecursively(dir_);
-  }
-
- private:
-  std::string dir_;
-};
-
-std::unique_ptr<ExtractTempDirectoryCleanup> cleanup_ptr;
+SapiLibarchiveSandboxExtract* sandbox_extract;
+LibarchiveApi* api;
+char* c_str_tmp = nullptr;
 
 typedef void (*real_extract)(const char*, int, int, int);
 
@@ -46,16 +35,15 @@ void extract(const char* filename, int do_extract, int flags, int verbose) {
   std::string tmp_dir;
   if (do_extract) {
     tmp_dir = CreateTempDirAtCWD().value();
-    cleanup_ptr = absl::make_unique<ExtractTempDirectoryCleanup>(tmp_dir);
   }
 
   std::string filename_absolute = MakeAbsolutePathAtCWD(filename);
 
   // Initialize sandbox and api objects.
-  sandbox_extract = absl::make_unique<SapiLibarchiveSandboxExtract>(
-      filename_absolute, do_extract, tmp_dir);
+  sandbox_extract =
+      new SapiLibarchiveSandboxExtract(filename_absolute, do_extract, tmp_dir);
   CHECK(sandbox_extract->Init().ok()) << "Error during sandbox initialization";
-  api = absl::make_unique<LibarchiveApi>(sandbox_extract.get());
+  api = new LibarchiveApi(sandbox_extract);
 
   // After everything is set up, call the original function (next symbol).
 
@@ -66,6 +54,20 @@ void extract(const char* filename, int do_extract, int flags, int verbose) {
 
   CHECK(x != nullptr) << "dlsym call could not find function symbol";
   ((real_extract)x)(filename_absolute.c_str(), do_extract, flags, verbose);
+
+  // clean up
+  if (c_str_tmp != nullptr) {
+    delete[] c_str_tmp;
+  }
+
+  // This is the last function called so we can delete the temporary directory
+  // here
+  if (do_extract) {
+    sandbox2::file_util::fileops::DeleteRecursively(tmp_dir);
+  }
+
+  delete api;
+  delete sandbox_extract;
 }
 
 archive* archive_read_new() {
@@ -126,13 +128,6 @@ int archive_read_open_filename(archive* a, const char* _filename,
       .value();
 }
 
-const char* archive_error_string(archive* a) {
-  sapi::v::RemotePtr a_ptr(a);
-  char* str = api->archive_error_string(&a_ptr).value();
-  CHECK(str != nullptr) << "Could not get error message";
-  return sandbox_extract->GetCString(sapi::v::RemotePtr(str)).value().c_str();
-}
-
 int archive_read_next_header(archive* a, archive_entry** entry) {
   sapi::v::IntBase<archive_entry*> entry_ptr_tmp(0);
   sapi::v::RemotePtr a_ptr(a);
@@ -142,17 +137,44 @@ int archive_read_next_header(archive* a, archive_entry** entry) {
   return rc;
 }
 
-// Keep a global variable so that it does not go out of scope.
-std::string str_tmp;
+// In the following two functions we need to transfer a string from the
+// sandboxed process to the client process. However, this string would
+// go out of scope after this function so we use a global char * to make
+// sure it does not get automatically deleted before it is used.
+const char* archive_error_string(archive* a) {
+  sapi::v::RemotePtr a_ptr(a);
+  char* str = api->archive_error_string(&a_ptr).value();
+  CHECK(str != nullptr) << "Could not get error message";
+
+  std::string str_tmp =
+      sandbox_extract->GetCString(sapi::v::RemotePtr(str)).value();
+
+  if (c_str_tmp != nullptr) {
+    delete[] c_str_tmp;
+  }
+
+  c_str_tmp = new char[str_tmp.length() + 1];
+  strcpy(c_str_tmp, str_tmp.c_str());
+
+  return c_str_tmp;
+}
 
 const char* archive_entry_pathname(archive_entry* entry) {
   sapi::v::RemotePtr entry_ptr(entry);
   char* str = api->archive_entry_pathname(&entry_ptr).value();
-  CHECK(str != nullptr) << "Could not get error message";
+  CHECK(str != nullptr) << "Could not get pathname";
 
-  str_tmp = sandbox_extract->GetCString(sapi::v::RemotePtr(str)).value();
+  std::string str_tmp =
+      sandbox_extract->GetCString(sapi::v::RemotePtr(str)).value();
 
-  return str_tmp.c_str();
+  if (c_str_tmp != nullptr) {
+    delete[] c_str_tmp;
+  }
+
+  c_str_tmp = new char[str_tmp.length() + 1];
+  strcpy(c_str_tmp, str_tmp.c_str());
+
+  return c_str_tmp;
 }
 
 int archive_read_close(archive* a) {