From 71cd49ea82313f78d52a52d0c628a3770dc96608 Mon Sep 17 00:00:00 2001 From: TJ Saunders Date: Wed, 17 Jul 2019 09:25:31 -0700 Subject: [PATCH] Bug #4372: Ensure that mod_copy checks for for its SITE CPFR/CPTO commands. https://tbspace.de/cve201912815proftpd.html Downloaded from upstream commit https://github.com/proftpd/proftpd/commit/d19dd64161936d70c0a1544bd2c8e90850f4b7ae Signed-off-by: Bernd Kuhls --- contrib/mod_copy.c | 36 ++- tests/t/lib/ProFTPD/Tests/Modules/mod_copy.pm | 253 +++++++++++++++++- 2 files changed, 285 insertions(+), 4 deletions(-) diff --git a/contrib/mod_copy.c b/contrib/mod_copy.c index 26b72a91d..c8672c40d 100644 --- a/contrib/mod_copy.c +++ b/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 }, diff --git a/tests/t/lib/ProFTPD/Tests/Modules/mod_copy.pm b/tests/t/lib/ProFTPD/Tests/Modules/mod_copy.pm index 778bff839..2018e71bc 100644 --- a/tests/t/lib/ProFTPD/Tests/Modules/mod_copy.pm +++ b/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 < @@ -3256,7 +3271,7 @@ sub copy_config_limit_bug3399 { - + AllowAll @@ -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 < + + DenyAll + + +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 < + + DenyAll + + +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;