Merge pull request proftpd#1297 from proftpd/limit-help-issue1296

Issue proftpd#1296: Support <Limit> configurations for the `HELP`, `SITE HEL…

modules/mod_core.c

@@ -4694,6 +4694,23 @@ MODRET core_epsv(cmd_rec *cmd) {
 }
 
 MODRET core_help(cmd_rec *cmd) {
+  if (!dir_check(cmd->tmp_pool, cmd, cmd->group, session.vwd, NULL)) {
+    int xerrno = EACCES;
+
+    pr_log_debug(DEBUG7, "%s command denied by <Limit> configuration",
+      (char *) cmd->argv[0]);
+
+    /* Returning 501 is the best we can do.  It would be nicer if RFC959 allowed
+     * 550 as a possible response.
+     */
+    pr_response_add_err(R_501, "%s: %s", (char *) cmd->argv[0],
+      strerror(xerrno));
+
+    pr_cmd_set_errno(cmd, xerrno);
+    errno = xerrno;
+    return PR_ERROR(cmd);
+  }
+
   if (cmd->argc == 1) {
     pr_help_add_response(cmd, NULL);
 

modules/mod_site.c

@@ -1,7 +1,7 @@
 /*
  * ProFTPD - FTP server daemon
  * Copyright (c) 1997, 1998 Public Flood Software
- * Copyright (c) 2001-2020 The ProFTPD Project team
+ * Copyright (c) 2001-2021 The ProFTPD Project team
  *
  * 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
@@ -480,6 +480,22 @@ MODRET site_help(cmd_rec *cmd) {
    * syntactically correct.
    */
 
+  if (!dir_check_limits(cmd, session.dir_config, "SITE_HELP", FALSE)) {
+    int xerrno = EACCES;
+
+    pr_log_debug(DEBUG8, "SITE HELP denied by <Limit> configuration");
+
+    /* Returning 501 is the best we can do.  It would be nicer if RFC959 allowed
+     * 550 as a possible response.
+     */
+    pr_response_add_err(R_501, "%s: %s", (char *) cmd->argv[0],
+      strerror(xerrno));
+
+    pr_cmd_set_errno(cmd, xerrno);
+    errno = xerrno;
+    return PR_ERROR(cmd);
+  }
+
   if (cmd->argc == 1 || (cmd->argc == 2 &&
       ((strcasecmp(cmd->argv[0], "SITE") == 0 &&
         strcasecmp(cmd->argv[1], "HELP") == 0) ||

tests/t/config/limit/help.t

@@ -0,0 +1,11 @@
+#!/usr/bin/env perl
+
+use lib qw(t/lib);
+use strict;
+
+use Test::Unit::HarnessUnit;
+
+$| = 1;
+
+my $r = Test::Unit::HarnessUnit->new();
+$r->start("ProFTPD::Tests::Config::Limit::HELP");

tests/t/lib/ProFTPD/Tests/Config/Limit/HELP.pm

@@ -0,0 +1,255 @@
+package ProFTPD::Tests::Config::Limit::HELP;
+
+use lib qw(t/lib);
+use base qw(ProFTPD::TestSuite::Child);
+use strict;
+
+use File::Path qw(mkpath);
+use File::Spec;
+use IO::Handle;
+
+use ProFTPD::TestSuite::FTP;
+use ProFTPD::TestSuite::Utils qw(:auth :config :running :test :testsuite);
+
+$| = 1;
+
+my $order = 0;
+
+my $TESTS = {
+  limit_help_issue1296 => {
+    order => ++$order,
+    test_class => [qw(forking)],
+  },
+
+  limit_site_help_issue1296 => {
+    order => ++$order,
+    test_class => [qw(forking)],
+  },
+
+};
+
+sub new {
+  return shift()->SUPER::new(@_);
+}
+
+sub list_tests {
+  return testsuite_get_runnable_tests($TESTS);
+}
+
+sub limit_help_issue1296 {
+  my $self = shift;
+  my $tmpdir = $self->{tmpdir};
+  my $setup = test_setup($tmpdir, 'limit');
+
+  my $config = {
+    PidFile => $setup->{pid_file},
+    ScoreboardFile => $setup->{scoreboard_file},
+    SystemLog => $setup->{log_file},
+    TraceLog => $setup->{log_file},
+    Trace => 'command:10 directory:10',
+
+    AuthUserFile => $setup->{auth_user_file},
+    AuthGroupFile => $setup->{auth_group_file},
+    DefaultChdir => '~',
+
+    IfModules => {
+      'mod_delay.c' => {
+        DelayEngine => 'off',
+      },
+    },
+
+    Limit => {
+      HELP => {
+        DenyAll => '',
+      },
+    }
+  };
+
+  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 {
+      my $client = ProFTPD::TestSuite::FTP->new('127.0.0.1', $port);
+
+      eval { $client->help() };
+      unless ($@) {
+        die("HELP command succeeded unexpectedly");
+      }
+
+      my $resp_code = $client->response_code();
+      my $resp_msg = $client->response_msg();
+
+      my $expected = 501;
+      $self->assert($expected == $resp_code,
+        test_msg("Expected response code $expected, got $resp_code"));
+
+      $expected = 'HELP: Permission denied';
+      $self->assert($expected eq $resp_msg,
+        test_msg("Expected response message '$expected', got '$resp_msg'"));
+
+      $client->login($setup->{user}, $setup->{passwd});
+      eval { $client->help() };
+      unless ($@) {
+        die("HELP command succeeded unexpectedly");
+      }
+
+      $resp_code = $client->response_code();
+      $resp_msg = $client->response_msg();
+
+      $expected = 501;
+      $self->assert($expected == $resp_code,
+        test_msg("Expected response code $expected, got $resp_code"));
+
+      $expected = 'HELP: Permission denied';
+      $self->assert($expected eq $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) };
+    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 limit_site_help_issue1296 {
+  my $self = shift;
+  my $tmpdir = $self->{tmpdir};
+  my $setup = test_setup($tmpdir, 'limit');
+
+  my $config = {
+    PidFile => $setup->{pid_file},
+    ScoreboardFile => $setup->{scoreboard_file},
+    SystemLog => $setup->{log_file},
+    TraceLog => $setup->{log_file},
+    Trace => 'command:10 directory:10',
+
+    AuthUserFile => $setup->{auth_user_file},
+    AuthGroupFile => $setup->{auth_group_file},
+    DefaultChdir => '~',
+
+    IfModules => {
+      'mod_delay.c' => {
+        DelayEngine => 'off',
+      },
+    },
+
+    Limit => {
+      SITE_HELP => {
+        DenyAll => '',
+      },
+    }
+  };
+
+  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 {
+      my $client = ProFTPD::TestSuite::FTP->new('127.0.0.1', $port);
+
+      eval { $client->site('help') };
+      unless ($@) {
+        die("SITE HELP command succeeded unexpectedly");
+      }
+
+      my $resp_code = $client->response_code();
+      my $resp_msg = $client->response_msg();
+
+      my $expected = 501;
+      $self->assert($expected == $resp_code,
+        test_msg("Expected response code $expected, got $resp_code"));
+
+      $expected = 'HELP: Permission denied';
+      $self->assert($expected eq $resp_msg,
+        test_msg("Expected response message '$expected', got '$resp_msg'"));
+
+      $client->login($setup->{user}, $setup->{passwd});
+      eval { $client->site('help') };
+      unless ($@) {
+        die("SITE HELP command succeeded unexpectedly");
+      }
+
+      $resp_code = $client->response_code();
+      $resp_msg = $client->response_msg();
+
+      $expected = 501;
+      $self->assert($expected == $resp_code,
+        test_msg("Expected response code $expected, got $resp_code"));
+
+      $expected = 'HELP: Permission denied';
+      $self->assert($expected eq $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) };
+    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;