--- 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;