Bug #4372: Ensure that mod_copy checks for <Limits> for its SITE CPFR…

…/CPTO

commands.

contrib/mod_copy.c

@@ -1,7 +1,7 @@
 /*
  * ProFTPD: mod_copy -- a module supporting copying of files on the server
  *                      without transferring the data to the client and back
- * Copyright (c) 2009-2016 TJ Saunders
+ * Copyright (c) 2009-2019 TJ Saunders
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -657,7 +657,7 @@ MODRET copy_copy(cmd_rec *cmd) {
 MODRET copy_cpfr(cmd_rec *cmd) {
   register unsigned int i;
   int res;
-  char *path = "";
+  char *cmd_name, *path = "";
   unsigned char *authenticated = NULL;
 
   if (copy_engine == FALSE) {
@@ -705,6 +705,21 @@ MODRET copy_cpfr(cmd_rec *cmd) {
     path = pstrcat(cmd->tmp_pool, path, *path ? " " : "", decoded_path, NULL);
   }
 
+  cmd_name = cmd->argv[0];
+  pr_cmd_set_name(cmd, "SITE_CPFR");
+  if (!dir_check(cmd->tmp_pool, cmd, G_READ, path, NULL)) {
+    int xerrno = EPERM;
+
+    pr_cmd_set_name(cmd, cmd_name);
+    pr_response_add_err(R_550, "%s: %s", (char *) cmd->argv[3],
+      strerror(xerrno));
+
+    pr_cmd_set_errno(cmd, xerrno);
+    errno = xerrno;
+    return PR_ERROR(cmd);
+  }
+  pr_cmd_set_name(cmd, cmd_name);
+
   res = pr_filter_allow_path(CURRENT_CONF, path);
   switch (res) {
     case 0:
@@ -758,6 +773,7 @@ MODRET copy_cpfr(cmd_rec *cmd) {
 MODRET copy_cpto(cmd_rec *cmd) {
   register unsigned int i;
   const char *from, *to = "";
+  char *cmd_name;
   unsigned char *authenticated = NULL;
 
   if (copy_engine == FALSE) {
@@ -816,6 +832,20 @@ MODRET copy_cpto(cmd_rec *cmd) {
 
   to = dir_canonical_vpath(cmd->tmp_pool, to);
 
+  cmd_name = cmd->argv[0];
+  pr_cmd_set_name(cmd, "SITE_CPTO");
+  if (!dir_check(cmd->tmp_pool, cmd, G_WRITE, to, NULL)) {
+    int xerrno = EPERM;
+
+    pr_cmd_set_name(cmd, cmd_name);
+    pr_response_add_err(R_550, "%s: %s", to, strerror(xerrno));
+
+    pr_cmd_set_errno(cmd, xerrno);
+    errno = xerrno;
+    return PR_ERROR(cmd);
+  }
+  pr_cmd_set_name(cmd, cmd_name);
+
   if (copy_paths(cmd->tmp_pool, from, to) < 0) {
     int xerrno = errno;
     const char *err_code = R_550;
@@ -940,7 +970,7 @@ static conftable copy_conftab[] = {
 
 static cmdtable copy_cmdtab[] = {
   { CMD, 	C_SITE, G_WRITE,	copy_copy,	FALSE,	FALSE, CL_MISC },
-  { CMD, 	C_SITE, G_DIRS,		copy_cpfr,	FALSE,	FALSE, CL_MISC },
+  { CMD, 	C_SITE, G_READ,		copy_cpfr,	FALSE,	FALSE, CL_MISC },
   { CMD, 	C_SITE, G_WRITE,	copy_cpto,	FALSE,	FALSE, CL_MISC },
   { POST_CMD,	C_PASS,	G_NONE,		copy_post_pass, FALSE,	FALSE },
   { LOG_CMD, 	C_SITE, G_NONE,		copy_log_site,	FALSE,	FALSE },

tests/t/lib/ProFTPD/Tests/Modules/mod_copy.pm

@@ -121,6 +121,15 @@ my $TESTS = {
     test_class => [qw(bug forking)],
   },
 
+  copy_cpfr_config_limit_read_bug4372 => {
+    order => ++$order,
+    test_class => [qw(bug forking)],
+  },
+
+  copy_cpto_config_limit_write_bug4372 => {
+    order => ++$order,
+    test_class => [qw(bug forking)],
+  },
 };
 
 sub new {
@@ -3248,6 +3257,12 @@ sub copy_config_limit_bug3399 {
 
   my ($port, $config_user, $config_group) = config_write($config_file, $config);
 
+  my $config_subdir = $sub_dir;
+  if ($^O eq 'darwin') {
+    # MacOSX hack
+    $config_subdir = '/private' . $sub_dir;
+  }
+
   if (open(my $fh, ">> $config_file")) {
     print $fh <<EOC;
 <Directory />
@@ -3256,7 +3271,7 @@ sub copy_config_limit_bug3399 {
   </Limit>
 </Directory>
 
-<Directory $sub_dir>
+<Directory $config_subdir>
   <Limit WRITE>
     AllowAll
   </Limit>
@@ -3652,4 +3667,240 @@ sub copy_cpto_timeout_bug4263 {
   test_cleanup($setup->{log_file}, $ex);
 }
 
+sub copy_cpfr_config_limit_read_bug4372 {
+  my $self = shift;
+  my $tmpdir = $self->{tmpdir};
+  my $setup = test_setup($tmpdir, 'copy');
+
+  my $src_file = File::Spec->rel2abs("$tmpdir/foo.dat");
+  if (open(my $fh, "> $src_file")) {
+    unless (close($fh)) {
+      die("Can't write $src_file: $!");
+    }
+
+  } else {
+    die("Can't open $src_file: $!");
+  }
+
+  my $config = {
+    PidFile => $setup->{pid_file},
+    ScoreboardFile => $setup->{scoreboard_file},
+    SystemLog => $setup->{log_file},
+    TraceLog => $setup->{log_file},
+    Trace => 'copy:20 timer:20',
+
+    AuthUserFile => $setup->{auth_user_file},
+    AuthGroupFile => $setup->{auth_group_file},
+    TimeoutIdle => 3,
+
+    IfModules => {
+      'mod_delay.c' => {
+        DelayEngine => 'off',
+      },
+    },
+  };
+
+  my ($port, $config_user, $config_group) = config_write($setup->{config_file},
+    $config);
+
+  if (open(my $fh, ">> $setup->{config_file}")) {
+    print $fh <<EOC;
+<Directory />
+  <Limit READ>
+    DenyAll
+  </Limit>
+</Directory>
+EOC
+    unless (close($fh)) {
+      die("Can't write $setup->{config_file}: $!");
+    }
+
+  } else {
+    die("Can't open $setup->{config_file}: $!");
+  }
+
+  # 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 {
+      my $client = ProFTPD::TestSuite::FTP->new('127.0.0.1', $port, 0, 1);
+      $client->login($setup->{user}, $setup->{passwd});
+
+      eval { $client->site('CPFR', 'foo.dat') };
+      unless ($@) {
+        die("SITE CPFR succeeded unexpectedly");
+      }
+
+      my $resp_code = $client->response_code();
+      my $resp_msg = $client->response_msg();
+
+      my $expected = 550;
+      $self->assert($expected == $resp_code,
+        test_msg("Expected response code $expected, got $resp_code"));
+
+      $expected = 'Operation not permitted';
+      $self->assert(qr/$expected/, $resp_msg,
+        test_msg("Expected response message '$expected', got '$resp_msg'"));
+
+      $client->quit();
+    };
+    if ($@) {
+      $ex = $@;
+    }
+
+    $wfh->print("done\n");
+    $wfh->flush();
+
+  } else {
+    eval { server_wait($setup->{config_file}, $rfh, 30) };
+    if ($@) {
+      warn($@);
+      exit 1;
+    }
+
+    exit 0;
+  }
+
+  # Stop server
+  server_stop($setup->{pid_file});
+  $self->assert_child_ok($pid);
+
+  test_cleanup($setup->{log_file}, $ex);
+}
+
+sub copy_cpto_config_limit_write_bug4372 {
+  my $self = shift;
+  my $tmpdir = $self->{tmpdir};
+  my $setup = test_setup($tmpdir, 'copy');
+
+  my $src_file = File::Spec->rel2abs("$tmpdir/foo.dat");
+  if (open(my $fh, "> $src_file")) {
+    unless (close($fh)) {
+      die("Can't write $src_file: $!");
+    }
+
+  } else {
+    die("Can't open $src_file: $!");
+  }
+
+  my $dst_file = File::Spec->rel2abs("$tmpdir/bar.dat");
+
+  my $config = {
+    PidFile => $setup->{pid_file},
+    ScoreboardFile => $setup->{scoreboard_file},
+    SystemLog => $setup->{log_file},
+    TraceLog => $setup->{log_file},
+    Trace => 'copy:20 timer:20',
+
+    AuthUserFile => $setup->{auth_user_file},
+    AuthGroupFile => $setup->{auth_group_file},
+    TimeoutIdle => 3,
+
+    IfModules => {
+      'mod_delay.c' => {
+        DelayEngine => 'off',
+      },
+    },
+  };
+
+  my ($port, $config_user, $config_group) = config_write($setup->{config_file},
+    $config);
+
+  if (open(my $fh, ">> $setup->{config_file}")) {
+    print $fh <<EOC;
+<Directory />
+  <Limit WRITE>
+    DenyAll
+  </Limit>
+</Directory>
+EOC
+    unless (close($fh)) {
+      die("Can't write $setup->{config_file}: $!");
+    }
+
+  } else {
+    die("Can't open $setup->{config_file}: $!");
+  }
+
+  # 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 {
+      my $client = ProFTPD::TestSuite::FTP->new('127.0.0.1', $port, 0, 1);
+      $client->login($setup->{user}, $setup->{passwd});
+
+      my ($resp_code, $resp_msg) = $client->site('CPFR', 'foo.dat');
+
+      my $expected = 350;
+      $self->assert($expected == $resp_code,
+        test_msg("Expected response code $expected, got $resp_code"));
+
+      $expected = 'File or directory exists, ready for destination name';
+      $self->assert($expected eq $resp_msg,
+        test_msg("Expected response message '$expected', got '$resp_msg'"));
+
+      eval { $client->site('CPTO', 'bar.dat') };
+      unless ($@) {
+        die('SITE CPTO succeeded unexpectedly');
+      }
+
+      my $resp_code = $client->response_code();
+      my $resp_msg = $client->response_msg();
+
+      my $expected = 550;
+      $self->assert($expected == $resp_code,
+        test_msg("Expected response code $expected, got $resp_code"));
+
+      $expected = 'Operation not permitted';
+      $self->assert(qr/$expected/, $resp_msg,
+        test_msg("Expected response message '$expected', got '$resp_msg'"));
+
+      $client->quit();
+    };
+    if ($@) {
+      $ex = $@;
+    }
+
+    $wfh->print("done\n");
+    $wfh->flush();
+
+  } else {
+    eval { server_wait($setup->{config_file}, $rfh, 30) };
+    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;