Blob Blame History Raw
--- contrib/mod_vroot/path.c
+++ contrib/mod_vroot/path.c
@@ -225,18 +225,30 @@ char *vroot_realpath(pool *p, const char
   return real_path;
 }
 
-int vroot_path_lookup(pool *p, char *path, size_t pathlen, const char *dir,
+/* The given `vpath` buffer is the looked-up path for the given `path`. */
+int vroot_path_lookup(pool *p, char *vpath, size_t vpathsz, const char *path,
     int flags, char **alias_path) {
   char buf[PR_TUNABLE_PATH_MAX + 1], *bufp = NULL;
+  const char *cwd;
+
+  if (vpath == NULL ||
+      path == NULL) {
+    errno = EINVAL;
+    return -1;
+  }
 
   memset(buf, '\0', sizeof(buf));
-  memset(path, '\0', pathlen);
+  if (vpath != NULL &&
+      vpathsz > 0) {
+    memset(vpath, '\0', vpathsz);
+  }
 
-  if (strcmp(dir, ".") != 0) {
-    sstrncpy(buf, dir, sizeof(buf));
+  cwd = pr_fs_getcwd();
 
+  if (strcmp(path, ".") != 0) {
+    sstrncpy(buf, path, sizeof(buf));
   } else {
-    sstrncpy(buf, pr_fs_getcwd(), sizeof(buf));
+    sstrncpy(buf, cwd, sizeof(buf));
   }
 
   vroot_path_clean(buf);
@@ -244,7 +256,17 @@ int vroot_path_lookup(pool *p, char *pat
   bufp = buf;
 
   if (strncmp(bufp, vroot_base, vroot_baselen) == 0) {
-    bufp += vroot_baselen;
+    size_t len;
+
+    /* Attempt to handle cases like "/base/base" and "/base/basefoo", where
+     * the base is just "/base".
+     * See https://github.com/proftpd/proftpd/issues/1491
+     */
+    len = strlen(bufp);
+    if (len > vroot_baselen &&
+        bufp[vroot_baselen] == '/') {
+      bufp += vroot_baselen;
+    }
   }
 
 loop:
@@ -254,19 +276,19 @@ loop:
       bufp[1] == '.' &&
       (bufp[2] == '\0' ||
        bufp[2] == '/')) {
-    char *tmp = NULL;
+    char *ptr = NULL;
 
-    tmp = strrchr(path, '/');
-    if (tmp != NULL) {
-      *tmp = '\0';
+    ptr = strrchr(vpath, '/');
+    if (ptr != NULL) {
+      *ptr = '\0';
 
     } else {
-      *path = '\0';
+      *vpath = '\0';
     }
 
-    if (strncmp(path, vroot_base, vroot_baselen) == 0 ||
-         path[vroot_baselen] != '/') {
-      snprintf(path, pathlen, "%s/", vroot_base);
+    if (strncmp(vpath, vroot_base, vroot_baselen) == 0 ||
+         vpath[vroot_baselen] != '/') {
+      snprintf(vpath, vpathsz, "%s/", vroot_base);
     }
 
     if (bufp[0] == '.' &&
@@ -277,7 +299,7 @@ loop:
     }
 
   } else if (*bufp == '/') {
-    snprintf(path, pathlen, "%s/", vroot_base);
+    snprintf(vpath, vpathsz, "%s/", vroot_base);
     bufp += 1;
     goto loop;
 
@@ -319,19 +341,19 @@ loop:
     }
 
     buflen = strlen(bufp) + 1;
-    tmplen = strlen(path);
+    tmplen = strlen(vpath);
 
-    if (tmplen + buflen >= pathlen) {
+    if (tmplen + buflen >= vpathsz) {
       errno = ENAMETOOLONG;
       return -1;
     }
 
-    path[tmplen] = '/';
-    memcpy(path + tmplen + 1, bufp, buflen);
+    vpath[tmplen] = '/';
+    memcpy(vpath + tmplen + 1, bufp, buflen);
   }
 
   /* Clean any unnecessary characters added by the above processing. */
-  vroot_path_clean(path);
+  vroot_path_clean(vpath);
 
   if (!(flags & VROOT_LOOKUP_FL_NO_ALIAS)) {
     int alias_count;
@@ -346,7 +368,7 @@ loop:
        * aliases are found.
        */
       bufp = buf;
-      start_ptr = path;
+      start_ptr = vpath;
 
       while (start_ptr != NULL) {
         char *ptr = NULL;
@@ -376,11 +398,11 @@ loop:
               *alias_path, start_ptr);
           }
 
-          sstrncpy(path, src_path, pathlen);
+          sstrncpy(vpath, src_path, vpathsz);
 
           if (end_ptr != NULL) {
             /* Now tack on our suffix from the scratchpad. */
-            sstrcat(path, bufp, pathlen);
+            sstrcat(vpath, bufp, vpathsz);
           }
 
           break;
@@ -409,5 +431,11 @@ loop:
     }
   }
 
+  /* Note that logging the session.chroot_path here will not help; mod_vroot
+   * deliberately always sets that to just "/".
+   */
+  pr_trace_msg(trace_channel, 19,
+    "lookup: path = '%s', cwd = '%s', base = '%s', vpath = '%s'", path, cwd,
+    vroot_base, vpath);
   return 0;
 }
--- contrib/mod_vroot/t/lib/ProFTPD/Tests/Modules/mod_vroot.pm
+++ contrib/mod_vroot/t/lib/ProFTPD/Tests/Modules/mod_vroot.pm
@@ -355,13 +355,18 @@ my $TESTS = {
     test_class => [qw(bug forking)],
   },
 
-  # See:
-  #  https://github.com/proftpd/proftpd/issues/59
+  # See:  https://github.com/proftpd/proftpd/issues/59
   vroot_alias_enametoolong_bug59 => {
     order => ++$order,
     test_class => [qw(bug forking)],
   },
 
+  # See: https://github.com/proftpd/proftpd/issues/1491
+  vroot_root_paths_hidden_issue1491 => {
+    order => ++$order,
+    test_class => [qw(bug forking)],
+  },
+
 };
 
 sub new {
@@ -12112,4 +12117,202 @@ sub vroot_alias_enametoolong_bug59 {
   unlink($log_file);
 }
 
+sub vroot_root_paths_hidden_issue1491 {
+  my $self = shift;
+  my $tmpdir = $self->{tmpdir};
+  my $setup = test_setup($tmpdir, 'vroot');
+
+  # Note: the actual reproduction recipe for this issue requires the use
+  # of a single-component root, e.g. "/store".  However, I use the
+  # normal automatically generated temporary directory (of multiple path
+  # components) here, for the rest of the machinery; the `use_opt` variable
+  # can be used in the future to run this test using the short `/opt` directory
+  # as the DefaultRoot.  Doing so require that that `/opt` directory be
+  # created (and populated!) manually.
+
+  my $use_opt = 0;
+
+  my $root_dir;
+
+  if ($use_opt) {
+    $root_dir = File::Spec->rel2abs('/opt');
+
+  } else {
+    $root_dir = File::Spec->rel2abs("$tmpdir/opt");
+    mkpath($root_dir);
+
+    my $root_files = [qw(
+      not-opt
+      opt
+      optagain
+      opttest
+    )];
+
+    foreach my $root_file (@$root_files) {
+      my $path = File::Spec->rel2abs("$root_dir/$root_file");
+      next if -f $path;
+
+      if (open(my $fh, "> $path")) {
+        close($fh);
+
+      } else {
+        die("Can't open $path: $!");
+      }
+    }
+
+    if ($< == 0) {
+      unless (chmod(0755, $root_dir)) {
+        die("Can't set perms on $root_dir to 0755: $!");
+      }
+
+      unless (chown($setup->{uid}, $setup->{gid}, $root_dir)) {
+        die("Can't set owner of $root_dir to $setup->{uid}/$setup->{gid}: $!");
+      }
+    }
+  }
+
+  my $config = {
+    PidFile => $setup->{pid_file},
+    ScoreboardFile => $setup->{scoreboard_file},
+    SystemLog => $setup->{log_file},
+    TraceLog => $setup->{log_file},
+    Trace => 'fsio:20 vroot:20 vroot.path:20',
+
+    AuthUserFile => $setup->{auth_user_file},
+    AuthGroupFile => $setup->{auth_group_file},
+    AuthOrder => 'mod_auth_file.c',
+
+    ShowSymlinks => 'off',
+
+    IfModules => {
+      'mod_delay.c' => {
+        DelayEngine => 'off',
+      },
+
+      'mod_vroot.c' => {
+        VRootEngine => 'on',
+        VRootLog => $setup->{log_file},
+        DefaultRoot => $root_dir,
+      },
+    },
+  };
+
+  my ($port, $config_user, $config_group) = config_write($setup->{config_file},
+    $config);
+
+  # Open pipes, for use between the parent and child processes.  Specifically,
+  # the child will indicate when it's done with its test by writing a message
+  # to the parent.
+  my ($rfh, $wfh);
+  unless (pipe($rfh, $wfh)) {
+    die("Can't open pipe: $!");
+  }
+
+  my $ex;
+
+  # Fork child
+  $self->handle_sigchld();
+  defined(my $pid = fork()) or die("Can't fork: $!");
+  if ($pid) {
+    eval {
+      # Allow for server startup
+      sleep(1);
+
+      my $client = ProFTPD::TestSuite::FTP->new('127.0.0.1', $port);
+      $client->login($setup->{user}, $setup->{passwd});
+
+      my $conn = $client->list_raw();
+      unless ($conn) {
+        die("Failed to LIST: " . $client->response_code() . " " .
+          $client->response_msg());
+      }
+
+      my $buf;
+      $conn->read($buf, 8192, 30);
+      eval { $conn->close() };
+
+      my $resp_code = $client->response_code();
+      my $resp_msg = $client->response_msg();
+      $self->assert_transfer_ok($resp_code, $resp_msg);
+
+      $client->quit();
+
+      if ($ENV{TEST_VERBOSE}) {
+        print STDERR "# data:\n$buf\n";
+      }
+
+      # We have to be careful of the fact that readdir returns directory
+      # entries in an unordered fashion.
+      my $res = {};
+      my $lines = [split(/\n/, $buf)];
+      foreach my $line (@$lines) {
+        if ($line =~ /^\S+\s+\d+\s+\S+\s+\S+\s+.*?\s+(\S+)$/) {
+          $res->{$1} = 1;
+        }
+      }
+
+      unless (scalar(keys(%$res)) > 0) {
+        die("LIST data unexpectedly empty");
+      }
+
+      my $expected = {
+        'not-opt' => 1,
+        'opt' => 1,
+        'optagain' => 1,
+        'opttest' => 1,
+      };
+
+      my $ok = 1;
+      my $mismatch;
+      foreach my $name (keys(%$res)) {
+        unless (defined($expected->{$name})) {
+          $mismatch = $name;
+          $ok = 0;
+          last;
+        }
+      }
+
+      unless ($ok) {
+        die("Unexpected name '$mismatch' appeared in LIST data")
+      }
+
+      $ok = 1;
+
+      my $missing;
+      foreach my $name (keys(%$expected)) {
+        unless (defined($res->{$name})) {
+          $missing = $name;
+          $ok = 0;
+          last;
+        }
+      }
+
+      unless ($ok) {
+        die("Unexpected name '$missing' missing from LIST data")
+      }
+    };
+    if ($@) {
+      $ex = $@;
+    }
+
+    $wfh->print("done\n");
+    $wfh->flush();
+
+  } else {
+    eval { server_wait($setup->{config_file}, $rfh) };
+    if ($@) {
+      warn($@);
+      exit 1;
+    }
+
+    exit 0;
+  }
+
+  # Stop server
+  server_stop($setup->{pid_file});
+  $self->assert_child_ok($pid);
+
+  test_cleanup($setup->{log_file}, $ex);
+}
+
 1;