diff --git a/modules/mod_core.c b/modules/mod_core.c index 0905842569..e957a125ff 100644 --- a/modules/mod_core.c +++ b/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 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); diff --git a/modules/mod_site.c b/modules/mod_site.c index 1d42d8d5f7..50222ba4a1 100644 --- a/modules/mod_site.c +++ b/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 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) || diff --git a/tests/t/config/limit/help.t b/tests/t/config/limit/help.t new file mode 100644 index 0000000000..466a436e05 --- /dev/null +++ b/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"); diff --git a/tests/t/lib/ProFTPD/Tests/Config/Limit/HELP.pm b/tests/t/lib/ProFTPD/Tests/Config/Limit/HELP.pm new file mode 100644 index 0000000000..c0b7a10218 --- /dev/null +++ b/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;