[Libguestfs] [PATCH v2 8/9] v2v: -i ova: Sanity check hrefs to ensure they stay inside the tarball.

[Richard W.M. Jones]

Check for hrefs like "../../../../dev/sda", or dubious symlinks in the
tarball, which could escape from the tarball into the host system.

We use realpath(3) to do this.  Since this function does not work on
non-existent files, we must change the signature of get_file_ref so it
can return whether the file exists or not.
---
 v2v/input_ova.ml  |  6 +++++-
 v2v/parse_ova.ml  | 41 +++++++++++++++++++++++++++++++++++++++--
 v2v/parse_ova.mli |  7 +++----
 3 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
index fc8fde4bc..9f7bc64ff 100644
--- a/v2v/input_ova.ml
+++ b/v2v/input_ova.ml
@@ -79,7 +79,11 @@ class input_ova ova = object
     (* Convert the disk hrefs into qemu URIs. *)
     let qemu_uris = List.map (
       fun { href; compressed } ->
-        let file_ref = get_file_ref ova_t href in
+        let file_ref =
+          match get_file_ref ova_t href with
+          | Some f -> f
+          | None ->
+             error (f_"-i ova: OVF references file ‘%s’ which was not found in the OVA archive") href in
 
         match compressed, file_ref with
         | false, LocalFile filename ->
diff --git a/v2v/parse_ova.ml b/v2v/parse_ova.ml
index 6c4af4464..76fa8ce13 100644
--- a/v2v/parse_ova.ml
+++ b/v2v/parse_ova.ml
@@ -17,6 +17,7 @@
  *)
 
 open Printf
+open Unix
 
 open Std_utils
 open Tools_utils
@@ -305,9 +306,33 @@ let get_file_ref ({ top_dir; ova_type } as t) href =
   let ovf = get_ovf_file t in
   let ovf_folder = Filename.dirname ovf in
 
+  (* Since [href] comes from an untrusted source, we must ensure
+   * that it doesn't reference a path outside [top_dir].  An
+   * additional complication is that [href] is relative to
+   * the directory containing the OVF ([ovf_folder]).  A further
+   * complication is that the file might not exist at all.
+   *)
   match ova_type with
-  | Directory -> LocalFile (ovf_folder // href)
+  | Directory ->
+     let filename = ovf_folder // href in
+     let real_top_dir = Realpath.realpath top_dir in
+     (try
+        let filename = Realpath.realpath filename in
+        if not (String.is_prefix filename real_top_dir) then
+          error (f_"-i ova: invalid OVA file: path ‘%s’ references a file outside the archive") href;
+        Some (LocalFile filename)
+      with
+        Unix_error (ENOENT, "realpath", _) -> None
+     )
+
   | TarOptimized tar ->
+     (* Security: Since the only thing we will do with the computed
+      * filename is to call get_tar_offet_and_size, it doesn't
+      * matter if the filename is bogus or references some file
+      * on the filesystem outside the tarball.  Therefore we don't
+      * need to do any sanity checking here.
+      *)
+
      (*             (1)                 (2)
       * ovf:        <top_dir>/bar.ovf   <top_dir>/foo/bar.ovf
       * ovf_folder: <top_dir>           <top_dir>/foo
@@ -321,7 +346,19 @@ let get_file_ref ({ top_dir; ova_type } as t) href =
        )
        else if top_dir = ovf_folder then href (* 1 *)
        else assert false in
-     TarFile (tar, filename)
+
+     (* Does the file exist in the tarball? *)
+     let cmd = sprintf "tar tf %s %s >/dev/null 2>&1"
+                       (quote tar) (quote filename) in
+     debug "ova: testing if %s exists in %s" filename tar;
+     if Sys.command cmd = 0 then (
+       debug "ova: file exists";
+       Some (TarFile (tar, filename))
+     )
+     else (
+       debug "ova: file does not exist";
+       None
+     )
 
 let ws = PCRE.compile "\\s+"
 let re_tar_message = PCRE.compile "\\*\\* [^*]+ \\*\\*$"
diff --git a/v2v/parse_ova.mli b/v2v/parse_ova.mli
index 54df752ad..1ebf9022a 100644
--- a/v2v/parse_ova.mli
+++ b/v2v/parse_ova.mli
@@ -59,10 +59,9 @@ val get_manifest : t -> mf_record list
     verify them.  Also VMware-generated OVAs can return
     non-existent files in this list. *)
 
-val get_file_ref : t -> string -> file_ref
-(** Convert an OVF [href] into an actual file reference.
-
-    Note this does not check that the file really exists. *)
+val get_file_ref : t -> string -> file_ref option
+(** Convert an OVF [href] into an actual file reference.  Returns [None]
+    if the file does not exist. *)
 
 val get_tar_offet_and_size : string -> string -> int64 * int64
 (** [get_tar_offet_and_size tar filename] looks up file in the [tar]
-- 
2.16.2